Projected zenith convenience function#1904
Conversation
Such a diagram might be helpful, but no need in this PR IMHO. I'm not sure where it would live anyway. I don't think we currently have any diagrams in function-level documentation. I guess it could be in a gallery example (or a new User's Guide page) but unless it improves on the references, I'd say let's not bother :)
Is there a reason this function needs JIT more than the rest of pvlib does? We use
Slope-Aware Backtracking has a small validation dataset in a table at the end. It's not enough on its own for this PR, but it's at least a start, and can be easily extended in at least one way (subtract 180 from |
I can't make this work. Could you double check if that's supposed to happen? # test by changing array azimuth
psz = psz_func(
array_tilt,
array_azimuth-180,
timedata["Apparent Elevation"],
timedata["Solar Azimuth"],
)
assert_allclose(psz, -timedata["True-Tracking"], atol=1e-3) |
My mistake, I forgot those test cases used a nonzero |
|
I've left out whether the result is valid like |
Reported at: pvlib#1725 (comment) Confirmed via "Slope-Aware Backtracking for Single-Axis Trackers", paragraph after Eq. 1 Co-Authored-By: Mark Mikofski <bwana.marko@yahoo.com>
Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com>
Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com>
Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com>
Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com>
|
@kandersolar , GitHub does not allow me to reply to your last review message, so here it goes:
I agree it's a good point. It's difficult to understand exactly the vectors and the projection plane without reading it. In fact, such a recommendation would be a nice addition to the notes section.
I also agree with that. But regarding the suggested explanations, I think it would be better to describe the use cases with real examples in general. If I didn't already know about the internals and purposes, I wouldn't understand why I wouldn't change the current arguments definitions (almost due to the 1:1 relation with the paper). Maybe move your suggested clarifications to the notes section, although I'm not sure of how to integrate them. I'd like to clear up that I don't have a problem applying your suggestions straight forward if you want to, specially due to my lack of experience in PV and the language barrier. And that I hope this comment helps improving the quality of the PR and doesn't add noise to the conversation. |
|
I'd like to move forward this PR another bit, specially the docstring. Would you like me to push @kandersolar suggestions, or give another suggestions? Feel free to request anything from my side. |
|
Thanks @echedey-ls for keeping this going :) Describing some real use cases is a great idea, and I like your proposed text. What do you think about a combination of |
Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com>
|
I like that pretty much, thanks for the improvements. I will add the still inexistent You can check the docs out here. |
Fixed the link finally
|
I had some trouble finding the example's link. I can add a section link too but reading it all seems a better choice. From my side, this looks like it can be merged! |
kandersolar
left a comment
There was a problem hiding this comment.
One trivial suggestion so that the image doesn't take up so much webpage space, but otherwise LGTM!
Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>
|
Thanks for this contribution and the patience @echedey-ls :) |
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.Adds a function that calculates the projected solar zenith.
Pending tasks:
[ ] Add diagram?singleaxis, change it's implementation to useprojected_solar_zenith_angleand change tests accordingly.Test data generation script for the PSZ implementation port from `singleaxis` to `projected_solar_zenith_angle`
Documentation links
Link to function.