Correct usage of xtol in ghi_from_poa_driesse_2023 (#1971)

* Fix typo

* Add test

* why is always the linter

* Update test_irradiance.py

* Update v0.10.4.rst

* Am I faster that the linter?

* Again the linter

* Clear up comments

* Update irradiance.py

Co-Authored-By: Anton Driesse <[email protected]>

* Update test_irradiance.py

---------

Co-authored-by: Anton Driesse <[email protected]>

Author: echedey-ls

docs/sphinx/source/whatsnew/v0.10.4.rst

@@ -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
 ~~~~~~~

pvlib/irradiance.py

@@ -1557,8 +1557,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]
     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).
@@ -1593,13 +1593,16 @@ def ghi_from_poa_driesse_2023(surface_tilt, surface_azimuth,
     '''
     # Contributed by Anton Driesse (@adriesse), PV Performance Labs. Nov., 2023
 
+    if xtol <= 0:
+        raise ValueError(f"xtol too small ({xtol:g} <= 0)")
+
     ghi_from_poa_array = np.vectorize(_ghi_from_poa)
 
     ghi, conv, niter = ghi_from_poa_array(surface_tilt, surface_azimuth,
                                           solar_zenith, solar_azimuth,
                                           poa_global,
                                           dni_extra, airmass, albedo,
-                                          xtol=0.01)
+                                          xtol=xtol)
 
     if isinstance(poa_global, pd.Series):
         ghi = pd.Series(ghi, poa_global.index)

pvlib/tests/test_irradiance.py

@@ -782,7 +782,7 @@ def test_dirint_min_cos_zenith_max_zenith():
     assert_series_equal(out, expected, check_less_precise=True)
 
 
-def test_ghi_from_poa_driesse():
+def test_ghi_from_poa_driesse(mocker):
     # inputs copied from test_gti_dirint
     times = pd.DatetimeIndex(
         ['2014-06-24T06-0700', '2014-06-24T09-0700', '2014-06-24T12-0700'])
@@ -825,6 +825,22 @@ def test_ghi_from_poa_driesse():
     expected = [0, -1, 0]
     assert_allclose(expected, niter)
 
+    # test xtol argument
+    poa_global = pd.Series([20, 300, 1000], index=times)
+    # test exception
+    xtol = -3.14159  # negative value raises exception in scipy.optimize.bisect
+    with pytest.raises(ValueError, match=rf"xtol too small \({xtol:g} <= 0\)"):
+        output = irradiance.ghi_from_poa_driesse_2023(
+            surface_tilt, surface_azimuth, zenith, azimuth,
+            poa_global, dni_extra=1366.1, xtol=xtol)
+    # test propagation
+    xtol = 3.141592
+    bisect_spy = mocker.spy(irradiance, "bisect")
+    output = irradiance.ghi_from_poa_driesse_2023(
+        surface_tilt, surface_azimuth, zenith, azimuth,
+        poa_global, dni_extra=1366.1, xtol=xtol)
+    assert bisect_spy.call_args[1]["xtol"] == xtol
+
 
 def test_gti_dirint():
     times = pd.DatetimeIndex(