8000 Update irradiance.aoi to use more reliable formula by kandersolar · Pull Request #1191 · pvlib/pvlib-python · GitHub
[go: up one dir, main page]
Skip to content

Update irradiance.aoi to use more reliable formula #1191

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

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 1 commit
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
Prev Previous commit
Next Next commit
roll back to 2a0425b
  • Loading branch information
@kandersolar
kandersolar committed May 14, 2021
commit c2e6471bb15ad292ad70a3f5682deb2a8a8de894
2 changes: 1 addition & 1 deletion 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 @@ -110,7 +110,7 @@ Bug fixes
* 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` (:issue:`1185`, :pull:`1191`)
:py:func:`~pvlib.irradiance.aoi_projection` (:issue:`1185`, :pull:`1191`)

Testing
~~~~~~~
Expand Down
40 changes: 10 additions & 30 deletions pvlib/irradiance.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,12 +160,6 @@ def aoi_projection(surface_tilt, surface_azimuth, solar_zenith, solar_azimuth):

Input all angles in degrees.

.. warning::

For certain inputs, numerical round-off can cause this function
to produce values slightly greater than 1.0 when the surface
normal and sun position vectors are parallel.

Parameters
----------
surface_tilt : numeric
Expand All @@ -188,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 All @@ -196,24 +193,13 @@ def aoi_projection(surface_tilt, surface_azimuth, solar_zenith, solar_azimuth):
return projection


def _spherical_to_cartesian(zenith, azimuth):
sin_zen = tools.sind(zenith)
return np.array([sin_zen*tools.cosd(azimuth),
sin_zen*tools.sind(azimuth),
tools.cosd(zenith)]).T


def aoi(surface_tilt, surface_azimuth, solar_zenith, solar_azimuth):
"""
Calculates the angle of incidence of the solar vector on a surface.
This is the angle between the solar vector and the surface normal.

Input all angles in degrees.

.. versionchanged:: 0.9.0
Updated to a slower but more reliable formula that correctly handles
cases where the surface normal and solar position vectors are parallel.

Parameters
----------
surface_tilt : numeric
Expand All @@ -230,21 +216,15 @@ def aoi(surface_tilt, surface_azimuth, solar_zenith, solar_azimuth):
aoi : numeric
Angle of incidence in degrees.
"""
# see discussion in https://github.com/pvlib/pvlib-python/issues/1185
solar_vec = _spherical_to_cartesian(solar_zenith, solar_azimuth)
surface_vec = _spherical_to_cartesian(surface_tilt, surface_azimuth)

c_vec = solar_vec - surface_vec
c_sqrd = np.sum((c_vec*c_vec).T, axis=0)
c = np.sqrt(c_sqrd)

aoi = 2 * np.arctan(c / np.sqrt((1+(1+c))*((1-c)+1)))
aoi_value = np.rad2deg(aoi)
projection = aoi_projection(surface_tilt, surface_azimuth,
solar_zenith, solar_azimuth)
aoi_value = np.rad2deg(np.arccos(projection))

for arg in [surface_tilt, surface_azimuth, solar_zenith, solar_azimuth]:
if hasattr(arg, 'index'):
aoi_value = pd.Series(aoi_value, index=arg.index)
break
try:
aoi_value.name = 'aoi'
except AttributeError:
pass

return aoi_value

Expand Down
6 changes: 3 additions & 3 deletions pvlib/tests/test_irradiance.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -792,12 +792,12 @@ 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_precision():
def test_aoi_projection_precision():
# GH 1185
zenith = 89.26778228223463
azimuth = 60.932028605997004
aoi = irradiance.aoi(zenith, azimuth, zenith, azimuth)
assert aoi == 0
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
Expand Down
0