Conversation
* add _to_radians and is_rad to convert only if necessary * prefix all functions with get_ * add get_f_sky_pv, get_poa_sky_pv, get_ground_angle_tangent, get_f_gnd_pv, get_f_gnd_pv, get_poa_gnd_pv, etc. * update API ui Signed-off-by: Mark Mikofski <bwana.marko@yahoo.com>
* get_irradiance, output ordered dict or dataframe * finish updating get_poa_global_bifacial to transpose beam and diffuse for each side separately
…ion" - use tan(zenith) in solar projection math latex - implement gcr_prime and ground-sky-angles calculations - add stub for ground-diffuse view factor Signed-off-by: Mark Mikofski <bwana.marko@yahoo.com>
- calculate ground-sky angles to previous and next rows, assuming height is nonzero - calculate limits on ground where it can see the sky - calculate the view factor as a function of z on the ground to the sky - fix places where it still says degrees, bad, no! - add fixme for pv-sky view factor, still has wrong formula - add tests for angles from point z on the ground to tops of current row, and limits of previous and next rows - add a script to make the plot of ground-sky view factor versus z
|
@wholmgren FYI: looks like pandas and other versions not supported by python-3.5, not sure? |
- add comments, change names x->z - add TODO's to limit number of rows, and set row-type: 'first', 'last', or 'middle'
- was difference of angles, should be difference of cosines - also add TODO's to return VF versus point x on panel, and don't use averages
|
Cliff mentioned that in the ivtools pr (I think). I haven’t had a chance to
look into it. Probably should be addressed in a separate pr.
…On Wed, Jul 3, 2019 at 5:38 PM Mark Mikofski ***@***.***> wrote:
@wholmgren <https://github.com/wholmgren> FYI: looks like pandas and
other versions not supported by python-3.5
<https://travis-ci.org/pvlib/pvlib-python/jobs/554012857#L379>, not sure?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#717?email_source=notifications&email_token=ABBOER75RFF75U7MYDRY3DLP5VBAFA5CNFSM4HMBP67KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZGAEYQ#issuecomment-508297826>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABBOER4OIOK4O57A5QKOCZLP5VBAFANCNFSM4HMBP67A>
.
|
- change calc_fx_sky to calc_fz_sky since z is for ground and x is for pv surface - add docstring to calc_fz_sky and for ground_sky_diffuse_view_factor
Co-authored-by: Mark Mikofski <bwana.marko@yahoo.com>
Co-authored-by: Mark Mikofski <bwana.marko@yahoo.com>
|
Terminology issue: pvlib isn't consistent in terms referring to solar zenith and solar azimuth. The last set of changes here renamed two parameters: So pvlib isn't consistent in these terms, and in places is not specific. With that in mind I'm going to open a discussion about the |
|
In my own code have standardized on |
|
I think this is ready to merge. @wholmgren wanna do the honors? |
|
If no one has any objections I'll merge this today, COB, okay? |
| @@ -0,0 +1,831 @@ | |||
| r""" | |||
There was a problem hiding this comment.
This is a nice module docstring, but I don't think it's rendered anywhere. Did I miss it? I don't recall rendering any module docstrings (for better or worse). I do recall that it might not be super simple for our current documentation strategy. If so, perhaps the doc string could be copied into another place, like a new bifacial.rst or an example gallery py file?
There was a problem hiding this comment.
@cwhanse any comment on this? Seems like a good idea to me. I happy to move it if you agree. thx.
pvlib/bifacial/infinite_sheds.py
Outdated
| def get_irradiance_poa(surface_tilt, surface_azimuth, apparent_zenith, | ||
| azimuth, gcr, height, pitch, ghi, dhi, dni, |
There was a problem hiding this comment.
Given where #1403 is going, I suggest we go with solar_zenith and solar_azimuth like we do here
pvlib-python/pvlib/irradiance.py
Lines 304 to 305 in c0c46b4
There was a problem hiding this comment.
Should we wait until after #1403 is closed to merge this PR?
pvlib/bifacial/infinite_sheds.py
Outdated
|
|
||
| Returns | ||
| ------- | ||
| output : OrderedDict or DataFrame |
There was a problem hiding this comment.
A few years later, perhaps we just go with dict instead of OrderedDict?
There was a problem hiding this comment.
@wholmgren do you mean change only the docstring, or replace the use of OrderedDict with {}?
kandersolar
left a comment
There was a problem hiding this comment.
A few sphinx rendering issues
Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com>
Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com>
|
@wholmgren @kanderso-nrel I think I have addressed all comments. I'm satisfied that this PR is ready to merge. @mikofski want the honor of pushing the big green button? |
| surface_azimuth) | ||
| f_gnd_beam = 1.0 - np.minimum( | ||
| 1.0, gcr * np.abs(cosd(surface_tilt) + sind(surface_tilt) * tan_phi)) | ||
| np.where(solar_zenith > max_zenith, 0., f_gnd_beam) # [1], Eq. 4 |
There was a problem hiding this comment.
@mikofski I think this line has no effect as there is no return value.
There was a problem hiding this comment.
That's a point, well spotted @kdebrab ! Would you like to open a PR fixing it?
There was a problem hiding this comment.
This line is probably supposed to start with f_gnd_beam = and clip it at max zenith so values don’t blow up.
ICYMI @cwhanse @kandersolar
There was a problem hiding this comment.
these lines are still unchanged in main:
f_gnd_beam = 1.0 - np.minimum(
1.0, gcr * np.abs(cosd(surface_tilt) + sind(surface_tilt) * tan_phi))
np.where(solar_zenith > max_zenith, 0., f_gnd_beam) # [1], Eq. 4
return f_gnd_beam # 1 - min(1, abs()) < 1 alwayswas a PR forthcoming?
There was a problem hiding this comment.
@mikofski has not been fixed, should have its own issue.

[ ] Closes issue #xxxxdocs/sphinx/source/api.rstfor API changes.docs/sphinx/source/whatsnewfile for all changes.Implements "infinite sheds" model for irradiance on long, regular bifacial array (fixed or single-axis tracked) on horizontal ground.