-
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
Add diffuse self-shading functions #1017
Conversation
Test failures appear to be unrelated, a change in pandas: #1018 |
pvlib/shading.py
Outdated
return np.degrees(psi_avg) | ||
|
||
|
||
def passias_sky_diffuse(masking_angle): |
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.
Wondering if this signature creates space for other approaches to calculate self-shading. What if the arguments are tilt
and 'gcr' and passias_masking_angle
was private and called within this function? I doubt that the average "masking angle" is general among other models, I could be wrong.
A thought: how is this visible sky dome calculation done in bifacial_vf
? Maybe there's some commonality that could inform the interface here, although we wouldn't re-use code from bifacial_vf
. If I recall right, that calculation is done with view factors over small increments of angle.
I have a mild preference for sky_diffuse_passias
as the function name.
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 separated the two functions in anticipation of people wanting to use the "worst-case" angle like SAM does instead of Passias et al.'s average angle. That said, a function that just does 1 - cos(angle/2)^2
does seem strange.
Good idea about bifacial_vf; I haven't had an excuse to get familiar with it before now but am happy to have one.
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 looked at the SAM documentation. I think that the low level functions that calculates the average masking_angle
is useful, as would be a function that returns masking_angle
at a point along the row's surface. Maybe someone would want to use the masking_angle
in combination with other sky diffuse models.
Still chewing on the name passias_sky_diffuse
for the effects function.
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 added a masking_angle
function that calculates the angle for a given point and renamed passias_sky_diffuse
to sky_diffuse_passias
, but am happy to rename if you have a better alternative.
Maybe someone would want to use the masking_angle in combination with other sky diffuse models.
It occurs to me that the masking angle is also useful for comparing with solar position to determine if direct shading is relevant.
|
||
Notes | ||
----- | ||
The pvlib-python authors believe that Eqn. 9 in [1]_ is incorrect. |
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 equation below isn't equivalent to the published equation? Or, you couldn't reproduce the published equation [9]?
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 latter. The published equation [9] also gives different results from evaluating the integral numerically: https://gist.github.com/kanderso-nrel/2c6c3a1853338cdef5b4bbc67092ccc8
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'm persuaded. Your solution looks correct to me. I computed a few points on the k=1 and k=2 curves to check.
Panel tilt from horizontal [degrees]. | ||
|
||
gcr : float | ||
The ground coverage ratio of the array [unitless]. |
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 have a notion that gcr
is sometimes defined as gcr := width * cos(tilt) / pitch
for fixed-tilt arrays. Is that a common usage? If so, this parameter description and the one in masking_angle_passias
should clarify that width / pitch
is meant.
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.
In my experience the usual definition of ground coverage ratio is the ratio of module area to ground area, where the "ground area" is left somewhat ambiguous. That's the definition used in Pvsyst, Helioscope and pvlib.tracking.SingleAxisTracker
. I think we should stick with gcr = width / pitch
. I'd be interested to see where the ratio of projected horizontal width to pitch is used.
pvlib/shading.py
Outdated
the top of the shading row. SAM assumes the "worst-case" loss where the | ||
masking angle is calculated for the bottom of the array [1]_. In [2]_ | ||
the masking angle is calculated as the average across the module height. | ||
|
||
This function, as in [2]_, makes the assumption that sky diffuse | ||
irradiance is isotropic. |
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.
Entirely editorial - I'd swap the order of the last and next-to-last sentences, so that the focus stays on the Passias approach.
@wholmgren @mikofski any objections to merge? |
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.
LGTM too. I made one comment on a trig identity that might be useful, but probably not essential. This is a great addition. Thanks Kevin! |
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.
consider substituting the trig identity, but not a blocker.
Closes #xxxxdocs/sphinx/source/api.rst
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`
).This implements a modified version of the diffuse self-shading model in Passias and Kallbach (1984) "SHADING EFFECTS IN ROWS OF SOLAR CELL PANELS", which I believe has an error in the equation for the average masking angle (Eq 9). Here is a gist comparing the paper's equation with the one I derived, along with a numerical integration for comparison: https://gist.github.com/kanderso-nrel/2c6c3a1853338cdef5b4bbc67092ccc8. Note that I implemented two versions of the paper's equation to account for an ambiguous minus sign.
If anyone has a Mathematica license and can produce a cleaner version of that equation, I'd be happy to replace it -- that quick and dirty Maxima script in the docstring is about as much as I can do with symbolic solvers.