-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Linear shade gh1690 #1725
base: main
Are you sure you want to change the base?
Linear shade gh1690 #1725
Conversation
Projected pitch on ground is P/cos(g)
- move linear shade loss to shading module - don't use ternary, doesn't work on vectors, instead use np.where() - set cross axis default to zero - test vectors - update docs
- of github.com:mikofski/pvlib-python into linear_shade_gh1690
- applicable to other monolithic thin film like CIGS, not just CdTe - only when shade is perpendicular to scribe lines
…n into linear_shade_gh1690
@kandersolar & @williamhobbs thanks for the feedback! I've addressed it. Can you review again? |
👀 derivations and figures added to |
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.
Doesn't this function do the same as this existing function:
pvlib-python/pvlib/bifacial/infinite_sheds.py
Line 346 in 275e671
def _shaded_fraction(solar_zenith, solar_azimuth, surface_tilt, |
projected_solar_zenith : numeric | ||
Zenith angle in degrees of the solar vector projected into the plane | ||
perpendicular to the tracker axes. |
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.
It seems like it would be a lot easier for the user if the parameter was simply solar_zenith
and the calculation to the projected solar zenith could be done behind the scenes (there should be a function for that in pvlib somewhere).
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.
The calculation for psz requires at least one additional argument, the solar azimuth and is not a trivial calculation:
sp = pvl.solarposition.get_solarposition(times, latitude, longitude, ...)
proj_zenith = np.degrees(np.arctan2(
np.sin(np.radians(sp.azimuth))*np.sin(np.radians(sp.apparent_zenith)),
np.cos(np.radians(sp.apparent_zenith))))
It is already in pvlib here:
pvlib-python/pvlib/tracking.py
Line 432 in 275e671
wid = np.degrees(np.arctan2(xp, zp)) |
which made me discover a typo in the comments here:
pvlib-python/pvlib/tracking.py
Line 421 in 275e671
# vector (xp, yp, zp) in the (y, z) plane, which is normal to the panel and |
should be the (x, z) plane, which is corrected lower down here:
pvlib-python/pvlib/tracking.py
Line 429 in 275e671
# Calculate angle from x-y plane to projection of sun vector onto x-z plane |
Anyway, I agree, it would be helpful to have PSZ as it's own function. And if it already exists in addition to tracking.py:432
then we should consolidate. I think it's out of scope for this PR, though so I'll open another one for that.
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've created #1734 to propose a stand alone PSZ function, but in the process, realized that the existing use in tracking.py
actually calculates the solar vector as observed in the reference frame of the tracker axes, which may be tilted north or south. So this new function would need to be generalized to different reference frames.
I'm coming around to this idea. One instant advantage of this standalone function would be generalization of this linear shade loss function to arbitrary slopes that have both NS and EW tilt! Thanks! However, I still think it's best to address separately, then return to this function to enhance it with the axis tilt and the solar azimuth.
pvlib/shading.py
Outdated
def tracker_shaded_fraction(tracker_theta, gcr, projected_solar_zenith, | ||
cross_axis_slope=0): | ||
r""" | ||
Shade fraction (FS) for trackers with a common angle on an east-west slope. |
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.
Shade fraction (FS) for trackers with a common angle on an east-west slope. | |
Shade fraction for trackers with a common angle on an east-west slope. |
The acronym is never used anywhere
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.
Doesn't this function do the same as this existing function:
pvlib.bifacial.infinite_sheds._shaded_fraction()
They differ in that the new function allows the array to be on a cross axis slope. If we adapt the infinite sheds model to allow arbitrary orientation, then we could reuse this function, but that work was delayed due to some questions about where the horizon should be and what the sky and ground view factors would include. Until those issues are resolve and that enhancement is implemented, this new shaded fraction function is more generalized.
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.
The acronym is never used anywhere
It was the acronym that @williamhobbs used in #1689 and it's used on L269 in the source and L301 in the docstring example. Anyway, I'm not attached to it.
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 used "FS" for shaded fraction because Lorenzo 2011 used it, but I think it is confusing - should it be Fs, it's easily confused with "First Solar" in this context, etc.
@mikofski can you comment on the relationship between this shaded fraction calculation and the one in section 6 of Slope-Aware Backtracking? They seem quite similar in spirit, to me at least. One thing I notice is that the cross-axis slope currently seems to be a left-handed rotation (assuming it is defined on the same axis as the tracker and sun angles), see here: In [9]: tracker_shaded_fraction(tracker_theta=80, gcr=0.5, projected_solar_zenith=80, cross_axis_slope=0)
Out[9]: array(0.65270364)
In [10]: tracker_shaded_fraction(tracker_theta=80, gcr=0.5, projected_solar_zenith=80, cross_axis_slope=10)
Out[10]: array(1.)
In [11]: tracker_shaded_fraction(tracker_theta=80, gcr=0.5, projected_solar_zenith=80, cross_axis_slope=-10)
Out[11]: array(0.30540729) |
@AdamRJensen Did you tell me this week that if this used Semi-related, do we need an array azimuth input, or is this only suitable for trackers with 180 deg azimuth? My implementation only worked for that case, but it maybe isn't ideal. We do have some true-tracking projects with not-180 azimuths (e.g., 168 deg). This might be scope-creep... |
What is pending to do in this PR? It has become of interest to me because I'd like to contribute a losses model that benefits from this calculation. (Conversation at G Groups, from end of September & start of October) |
We got stuck considering consolidation & reuse of some private or intermediate calculations elsewhere (tracking & infinite sheds) that would either require extra work or would generalize this function to more relaxed conditions. Currently this is only good for N-S tracker (or possibly E-W fixed tilt) systems with perpendicular cross axis slopes. Another sticking point was whether user should have to calculate PSZ (projected solar zenith) or not. I opened a separate issue for that. So I’m not sure where we go from here. I think @williamhobbs submitted some PR’s recently that may have helped move this one forward as well? |
@echedey-ls, @mikofski is correct, this PR https://github.saobby.my.eu.org.mcas.ms/pvlib/pvlib-python/pull/1871 will let users get shaded fraction from |
@echedey-ls if you’re interested in helping resolve the blockers and pushing this across the finish line that would be great. This PR is still important to me specifically because it provides a r2r shading solution for sloping terrain. |
@mikofski I'm interested in it too for the linked conversation model, but I've got to have a deeper look at the discussion and the code. |
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>
Thanks! It’s okay to branch my branch, looking forward to seeing this closed finally! |
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>
* Function prototype * Update shading.rst * Update shading.py * Minimal test * Implementation From NREL paper * Fix, fix, fix, fix & format * Format issues * Extend tests (compare with singleaxis) & format with ruff * Format fixes * Upgrade tests * Array -> Axis * type * Whatsnew * xd * bruh * Minor Python optimization a la tracking.singleaxis * Comment and minor optimizations * Typo found by Mikofski Reported at: #1725 (comment) Confirmed via "Slope-Aware Backtracking for Single-Axis Trackers", paragraph after Eq. 1 Co-Authored-By: Mark Mikofski <bwana.marko@yahoo.com> * Surface -> Axis Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com> * Elevation -> Zenith Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com> * Elev -> Zenith Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com> * Update shading.py * Update docstring Co-Authored-By: Anton Driesse <9001027+adriesse@users.noreply.github.com> * Add comments from `tracking.singleaxis` Co-Authored-By: Will Holmgren <william.holmgren@gmail.com> Co-Authored-By: Mark Mikofski <bwana.marko@yahoo.com> * Singleaxis implementation port & test addition, based on old pvlib.tracking.singleaxis * Update v0.10.4.rst * Linter * Code review Co-Authored-By: Cliff Hansen <5393711+cwhanse@users.noreply.github.com> * Add Fig 5 [1] (still gotta check the built output) * Add caption, change size and describe in alternate text * rST fixes ? * Figures have captions, images do not https://pandemic-overview.readthedocs.io/en/latest/myGuides/reStructuredText-Images-and-Figures-Examples.html#id18 * Flip arguments order * I forgot 💀 * Linter are you happy now? * Remove port test and add edge cases test Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com> * Update test_shading.py Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com> * Indentation xd * Update test_shading.py * I forgot how to code * Align data * Docstring suggestion from Kevin Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com> * Update link to example? * Link, please work * Update shading.py * Update shading.py * Update shading.py * Update shading.py * Update shading.py * Update shading.py * Update shading.py * Update shading.py * Lintaaaaaaarrrgh Fixed the link finally * Update pvlib/shading.py Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com> --------- Co-authored-by: Mark Mikofski <bwana.marko@yahoo.com> Co-authored-by: Kevin Anderson <57452607+kandersolar@users.noreply.github.com> Co-authored-by: Anton Driesse <9001027+adriesse@users.noreply.github.com> Co-authored-by: Will Holmgren <william.holmgren@gmail.com> Co-authored-by: Cliff Hansen <5393711+cwhanse@users.noreply.github.com> Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>
docs/sphinx/source/reference
for API changes.docs/sphinx/source/whatsnew
for 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.add functions to get shaded fraction and calculate linear shade loss
ICYMI: @williamhobbs
Example: