Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Projected zenith convenience function #1904
Projected zenith convenience function #1904
Changes from 34 commits
1e412ce
e4a95a3
54878db
b79ebe6
c3c0e56
5d4a2b4
9ae5fa3
f17379d
af3d27c
d36841a
19995e1
770e037
29bcef9
61c2e3b
935cfd4
965b0b4
346d060
085d017
79b5f4f
dc1035a
88cbfc0
1afec94
4b2c0e5
4612442
6372cb7
3c9392b
93998d8
337f7f1
0923109
6955f71
939b241
b0d6f66
3df2e71
428852c
64c97c7
aed85a3
5d37fb3
38c2d4d
23aaa2a
2c0fa51
5b49706
1a68390
58d853f
2ba4cf7
8325c37
f2fcc88
069688e
c249224
f6c245f
b52f51d
96ce603
ff42463
1c28e7f
cd346e9
796c7a9
761750c
2be29f2
8beef55
5e2be60
3c5308b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does that mean the axis of tracker rotation is the surface normal vector we're talking about?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From Figure 5 caption:
I'm pretty sure it is. Do you think this line of text is worth to be added to the current caption? I didn't want to copy-paste the paper content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about:
That puts a picture in my mind!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's more focused on tangible parts, I like it.
Nonetheless, I'm a bit hesitant to change that since there is ongoing discussion regarding the tracking or fixed support nomenclature. And that suggests SAT.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries, I wasn't actually expecting this wording to be adopted. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a general purpose function, would
plane_tilt
be more appropriate? And similar for azimuth.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a comment up there from @kandersolar asking for changes from
surface_*
toaxis_*
prefixes. I thinksurface
is a close synonym ofplane
? So I'm unsure of applying that suggestion.I'll request reviews for the current reviewers hoping they also give some insight regarding this. Anyway, if you are sure of it, I change it without no problems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the issue that
axis_tilt
doesn't suggest that the equations are equally applicable to fixed tilt arrays?I'm inclined (pun intended) to keep the term
axis_tilt
here. I think it's a useful, although perhaps not obvious at first, point of view to consider FT as a subset/special case of SAT anyway, in which caseaxis_tilt
is the relevant quantity even for fixed tilt.Perhaps a good compromise is to clarify that point in the parameter description/Notes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like these two terms refer to pure maths, and mapping those words to concise real-world systems is a bit counter-productive due to the variability. Is there some generic -maths- term we can use for a general description of this, a projection? Like
basis
, maybe? There's always a beautiful open paper backing the procedure and explaining what it does thoroughly. How the tracker works is up to the user or the other functions.Just to weight in some more options outside of the current frame:
tracker_*
(but this suggests everything except FT, right?),collector_*
,reference_*
,system_*
,projection_system_*
,coordinate_system_*
,reference_system_*
,basis_*
,projection_basis_*
... whatever other constructs you may think of.I don't dislike this possibility if we don't agree on anything else eventually, but we are overcomplicating the docs IMHO.
BTW, I'm +1 for the
basis_*
,projection_basis_*
options. Anyway, remember I have 0 experience in PV outside of this repo.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I admit to suffering from tunnel vision for trackers, but in my view
axis_
is still the best suggested prefix thus far. I think no matter what prefix we choose, we're going to have to include some kind of explanation/clarification. So IMHO we might as well chooseaxis_
for consistency with the reference and other pvlib functions.How about:
Similar for
axis_azimuth
. And then a Notes section: