Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update irradiance.aoi to use more reliable formula #1191

Merged
merged 10 commits into from
May 14, 2021
Merged

Update irradiance.aoi to use more reliable formula #1191

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8069470
clip to [-1, 1]
kandersolar Mar 10, 2021
e0e7d99
add test
kandersolar Mar 10, 2021
2a0425b
whatsnew
kandersolar Mar 10, 2021
3ee6418
use new aoi formula
kandersolar Mar 13, 2021
8fb5070
revert changes to aoi_projection
kandersolar Mar 13, 2021
30cb9f2
update test and whatsnew
kandersolar Mar 13, 2021
671cf16
don't set name; break after first Series
kandersolar Mar 17, 2021
c2e6471
roll back to 2a0425b
kandersolar May 14, 2021
669483d
Merge remote-tracking branch 'upstream/master' into aoi_projection_clamp
kandersolar May 14, 2021
ab7b57d
test improvements
kandersolar May 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/sphinx/source/whatsnew/v0.9.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ Bug fixes
(:issue:`1065`, :pull:`1140`)
* Reindl model fixed to generate sky_diffuse=0 when GHI=0.
(:issue:`1153`, :pull:`1154`)
* Fix floating point round-off issue in
:py:func:`~pvlib.irradiance.aoi_projection` (:issue:`1185`, :pull:`1191`)
* Update GFS product names for GFS v16. (:issue:`1202`, :pull:`1203`)
* Corrected methodology error in :py:func:`~pvlib.scaling.wvm`. Tracks with
fix in PVLib for MATLAB. (:issue:`1206`, :pull:`1213`)
Expand Down
3 changes: 3 additions & 0 deletions pvlib/irradiance.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,9 @@ def aoi_projection(surface_tilt, surface_azimuth, solar_zenith, solar_azimuth):
tools.sind(surface_tilt) * tools.sind(solar_zenith) *
tools.cosd(solar_azimuth - surface_azimuth))

# GH 1185
projection = np.clip(projection, -1, 1)

try:
projection.name = 'aoi_projection'
except AttributeError:
Expand Down
8 changes: 8 additions & 0 deletions pvlib/tests/test_irradiance.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,14 @@ def test_aoi_and_aoi_projection(surface_tilt, surface_azimuth, solar_zenith,
assert_allclose(aoi_projection, aoi_proj_expected, atol=1e-6)


def test_aoi_projection_precision():
# GH 1185
zenith = 89.26778228223463
azimuth = 60.932028605997004
projection = irradiance.aoi_projection(zenith, azimuth, zenith, azimuth)
assert projection == 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know enough to intelligently comment on this, but I noticed that we're clipping to the int values -1 and 1. Is projection here also an int? I believe that if tested with realistic array input we'd always see floats returned. Might be worth testing that the clip behavior remains as expected in that case. Perhaps the important thing is that projection is always <= 1 and that np.isclose(projection, 1) returns True.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestions. I also added a dtype check to make sure that clipping with int limits doesn't do something funky.



@pytest.fixture
def airmass_kt():
# disc algorithm stopped at am=12. test am > 12 for out of range behavior
Expand Down