calculate surface angles using unit normal#2702
Conversation
| surface_azimuth = np.degrees( | ||
| np.arctan2(unit_normal[:, 0], unit_normal[:, 1])) |
There was a problem hiding this comment.
I would include this line in the function that calculates the unit normal so that it is self-contained w.r.t. its conventions.
There was a problem hiding this comment.
Are you suggesting that _unit_normal calculate and return surface_azimuth? I'm unclear what you are suggesting.
There was a problem hiding this comment.
That was the thought, yes. Angles in, angle out is independent of the method. I guess the function name would change too.
There was a problem hiding this comment.
Thanks for the suggestion. I guess I'm preferring _unit_normal as the function because it may find other uses. We could package the few lines here into a helper _calc_surface_azimuth but I don't see that as much of an improvement.
echedey-ls
left a comment
echedey-ls
left a comment
There was a problem hiding this comment.
I've compared _unit_normal with an implementation I did based on scipy rotation objects, and they match. The 3d axis basis is the same E-N-Up, left-hand rotations.
I just left this comment in the issue, I hope not to delay a fix on negative tilt handling.
pvlib/tracking.py
Outdated
| Unit normal to rotated tracker surface, in global E-N-Up coordinates, | ||
| given by R*(0, 0, 1)^T, where: | ||
|
|
||
| R = Rz(-axis_azimuth) Rx(-axis_tilt) Ry(theta) * |
There was a problem hiding this comment.
| R = Rz(-axis_azimuth) Rx(-axis_tilt) Ry(theta) * | |
| R = Rz(-axis_azimuth) Rx(-axis_tilt) Ry(theta) |
pvlib/tracking.py
Outdated
| rotation by -axis_tilt about the x-axis, where axis_tilt is negated | ||
| because pvlib's convention is that the positive y-axis is tilted | ||
| downwards. Ry is a rotation by theta | ||
| about the y-axis. theta is negated so that a negative. |
There was a problem hiding this comment.
I think this sentence was not ended correctly, right?
| example, for a tracker with ``axis_azimuth``=180 and ``axis_tilt``=10, | ||
| the north end is higher than the south end of the axis. | ||
|
|
||
| axis_azimuth : float, default 0 |
There was a problem hiding this comment.
All the examples refer to azimuth 180, so why is the default zero?
There was a problem hiding this comment.
I don't think there's a particular reason for the default of 0. It's the least common orientation in the northern hemisphere but perhaps its most common in the southern hemisphere. 0 has been the default for a long time, so maybe inertia wins? Examples are different question, though.
| @@ -84,7 +84,7 @@ def singleaxis(apparent_zenith, solar_azimuth, | |||
| intersection between the slope containing the tracker axes and a plane | |||
There was a problem hiding this comment.
I haven't looked at these functions much before, and it took me a while to realize that "slope" usually refers to the terrain or ground itself, or in this case surface floating above the ground, rather than an attribute of a line or surface. I the documentation could be improved by changing "slope" to "sloped surface" or "sloped ground"or "sloped terrain" or similar depending on the context.
If this gets a few thumbs up I can make specific suggestions.
There was a problem hiding this comment.
I agree that this description can be improved. "slope" is not specific. The precise definition should describe an intersection of two planes (because that's how a line is defined, and lines define angles). I think "slope" is intended to mean the plane that contains the tracker axes. @kandersolar
3 participants
docs/sphinx/source/referencefor API changes.docs/sphinx/source/whatsnewfor all changes. Includes link to the GitHub Issue with:issue:`num`or this Pull Request with:pull:`num`. Includes contributor name and/or GitHub username (link with:ghuser:`user`).remote-data) and Milestone are assigned to the Pull Request and linked Issue.Changes the calculation of surface angles for tracking.singleaxis to allow for negative axis_tilt.
Changes the output of singleaxis in the case of surface_tilt=0, to consistently return surface_azimuth=axis_azimuth.