Skip to content

add single axis tracking #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Apr 4, 2015
Merged

add single axis tracking #35

merged 16 commits into from
Apr 4, 2015

Conversation

wholmgren
Copy link
Member

This PR contains a Python implementation of PVLIB MATLAB's pvl_singleaxis.m. This isn't yet ready to merge (needs tests and PEP8), but I wanted to share progress and provide a place for feedback.

The code and comments are very similar to pvl_singleaxis.m. I think that some things can be simplified, but I wanted to get something working first. Sandia put a lot of comments in the code and I'm inclined to keep most of them.

I believe that the AOI calculation is correct, but there's something wrong with the surface azimuth and tilt calculation.

Here's an IPython notebook that explores most of the parameters.

@bmu bmu added this to the 0.2 milestone Mar 20, 2015
@wholmgren
Copy link
Member Author

A lot of changes since the initial PR.

  • Fixed the problem with surface_tilt and surface_azimuth. I'm not sure whether the MATLAB authors intended for surface_tilt to be with respect to surface horizontal plane or surface normal.
  • Parameters and most of the inline code has been PEP8-ified.
  • The MATLAB authors required a latitude input and threw an error if the latitude was less than 0, but I think the new code works fine independent of the latitude.
  • The sample notebook walks through each step of the algorithm and then tests the whole thing in many regimes.
  • I added some unit tests.

It would be helpful for somebody to make an initial review at this point.

@alorenzo175
Copy link
Contributor

What if the given elevation and azimuth don't have the same index? Could run into problems when doing things like finding wid. I would suggest ensuring/forcing apparent_zenith and apparent_azimuth to have the same index that you then use for times.

@wholmgren
Copy link
Member Author

@alorenzo175 pointed out that the surface_tilt calculation was done wrong, and that surface_azimuth did not work for non-integer multiples of 90 degrees. I think I've fixed both problems in the latest commit, and added some explanatory comments in the code and notebook. The second half of the surface_azimuth calculation is now significantly different than that of the MATLAB code.

I expect that the tests will fail since I didn't update the expected values. I'll make the other changes too.

@wholmgren
Copy link
Member Author

Updated the tests and added a few lines for @alorenzo175. Added introduction material to notebook.

@wholmgren
Copy link
Member Author

Added the same examples as the PVLIB MATLAB 1.2 help file to the tutorial. Updated internal variable names to be consistent with PEP8 and, I think, make the algorithm more understandable. Also replaced lots of arctan mapping code with arctan2.

I think this is close to being ready to merge.

@wholmgren
Copy link
Member Author

Rebased so that the test run against all of the pandas versions.

Removed the latitude kwarg since I don't think it's necessary.

I am happy to make changes, but if there are no objections to merging then let's go ahead and do it.

@bmu bmu modified the milestones: 0.1, 0.2 Apr 4, 2015
bmu added a commit that referenced this pull request Apr 4, 2015
@bmu bmu merged commit 92d6704 into pvlib:master Apr 4, 2015
@wholmgren wholmgren mentioned this pull request Apr 5, 2015
@wholmgren wholmgren deleted the tracking branch October 28, 2015 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants