Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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.10.4.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ Bug fixes
~~~~~~~~~
* :py:class:`~pvlib.modelchain.ModelChain` now raises a more useful error when
``temperature_model_parameters`` are specified on the passed ``system`` instead of on its ``arrays``. (:issue:`1759`).
* :py:func:`pvlib.irradiance.ghi_from_poa_driesse_2023` now correctly makes use
of the ``xtol`` argument. Previously, it was ignored. (:issue:`1970`, :pull:`1971`)

Testing
~~~~~~~
Expand Down
20 changes: 12 additions & 8 deletions pvlib/irradiance.py
Original file line number Diff line number Diff line change
Expand Up @@ -1513,11 +1513,15 @@ def poa_error(ghi):
full_output=True,
disp=False,
)
except ValueError:
# this occurs when poa_error has the same sign at both end points
ghi = np.nan
conv = False
niter = -1
except ValueError as e:
if "f(a) and f(b) must have different signs" in e.args:
# this occurs when poa_error has the same sign at both end points
ghi = np.nan
conv = False
niter = -1
else:
# propagate other ValueError's, like a non-positive xtol
raise e
else:
ghi = result[0]
conv = result[1].converged
Expand Down Expand Up @@ -1557,8 +1561,8 @@ def ghi_from_poa_driesse_2023(surface_tilt, surface_azimuth,
albedo : numeric, default 0.25
Ground surface albedo. [unitless]
xtol : numeric, default 0.01
Convergence criterion. The estimated GHI will be within xtol of the
true value. [W/m^2]
Convergence criterion. The estimated GHI will be within xtol of the
true value. Must be positive. [W/m^2]
Copy link
Member

Choose a reason for hiding this comment

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

Interestingly, the bisect docstring says nonnegative, which is inconsistent with its code.

full_output : boolean, default False
If full_output is False, only ghi is returned, otherwise the return
value is (ghi, converged, niter). (see Returns section for details).
Expand Down Expand Up @@ -1599,7 +1603,7 @@ def ghi_from_poa_driesse_2023(surface_tilt, surface_azimuth,
solar_zenith, solar_azimuth,
poa_global,
dni_extra, airmass, albedo,
xtol=0.01)
xtol=xtol)
Copy link
Member

Choose a reason for hiding this comment

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

Good catch!


if isinstance(poa_global, pd.Series):
ghi = pd.Series(ghi, poa_global.index)
Expand Down
8 changes: 8 additions & 0 deletions pvlib/tests/test_irradiance.py
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,14 @@ def test_ghi_from_poa_driesse():
expected = [0, -1, 0]
assert_allclose(expected, niter)

# test xtol propagation by producing an exception
poa_global = pd.Series([20, 300, 1000], index=times)
xtol = -3.14159 # negative value raises exception in scipy.optimize.bisect
with pytest.raises(ValueError, match=r"xtol too small \(%g <= 0\)" % xtol):
output = irradiance.ghi_from_poa_driesse_2023(
surface_tilt, surface_azimuth, zenith, azimuth,
poa_global, dni_extra=1366.1, xtol=xtol)


def test_gti_dirint():
times = pd.DatetimeIndex(
Expand Down