From 049a02d51bf88f506394b2c525190ef4295e9105 Mon Sep 17 00:00:00 2001
From: echedey-ls <80125792+echedey-ls@users.noreply.github.com>
Date: Sat, 10 Feb 2024 22:59:18 +0100
Subject: [PATCH 01/10] Fix typo

---
 pvlib/irradiance.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pvlib/irradiance.py b/pvlib/irradiance.py
index 34098147ef..bf65b37680 100644
--- a/pvlib/irradiance.py
+++ b/pvlib/irradiance.py
@@ -1599,7 +1599,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)
 
     if isinstance(poa_global, pd.Series):
         ghi = pd.Series(ghi, poa_global.index)

From 0128477e7e21feae530367db0792540e76521d4f Mon Sep 17 00:00:00 2001
From: echedey-ls <80125792+echedey-ls@users.noreply.github.com>
Date: Sat, 10 Feb 2024 23:28:45 +0100
Subject: [PATCH 02/10] Add test

---
 pvlib/irradiance.py            | 13 ++++++++-----
 pvlib/tests/test_irradiance.py | 11 +++++++++++
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/pvlib/irradiance.py b/pvlib/irradiance.py
index bf65b37680..7dfd356d04 100644
--- a/pvlib/irradiance.py
+++ b/pvlib/irradiance.py
@@ -1513,11 +1513,14 @@ 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:
+        # this may occur because poa_error has the same sign at both end points
+        if "f(a) and f(b) must have different signs" in e.args:
+            ghi = np.nan
+            conv = False
+            niter = -1
+        else:
+            raise e
     else:
         ghi = result[0]
         conv = result[1].converged
diff --git a/pvlib/tests/test_irradiance.py b/pvlib/tests/test_irradiance.py
index 55fef490ba..fcb141b71a 100644
--- a/pvlib/tests/test_irradiance.py
+++ b/pvlib/tests/test_irradiance.py
@@ -1,6 +1,7 @@
 import datetime
 from collections import OrderedDict
 import warnings
+import re
 
 import numpy as np
 from numpy import array, nan
@@ -825,6 +826,16 @@ 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=re.escape("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(

From c8e4e18d064e1f35d8c9cc48896575427c531293 Mon Sep 17 00:00:00 2001
From: echedey-ls <80125792+echedey-ls@users.noreply.github.com>
Date: Sat, 10 Feb 2024 23:33:20 +0100
Subject: [PATCH 03/10] why is always the linter

---
 pvlib/tests/test_irradiance.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/pvlib/tests/test_irradiance.py b/pvlib/tests/test_irradiance.py
index fcb141b71a..96effb2ab6 100644
--- a/pvlib/tests/test_irradiance.py
+++ b/pvlib/tests/test_irradiance.py
@@ -831,7 +831,8 @@ def test_ghi_from_poa_driesse():
     xtol = -3.14159  # negative value raises exception in scipy.optimize.bisect
     with pytest.raises(
         ValueError,
-        match=re.escape("xtol too small (%g <= 0)" % xtol)):
+        match=re.escape("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)

From 5688c25d2a8e6a70afcbad53543cc7aff657fccb Mon Sep 17 00:00:00 2001
From: echedey-ls <80125792+echedey-ls@users.noreply.github.com>
Date: Sat, 10 Feb 2024 23:34:34 +0100
Subject: [PATCH 04/10] Update test_irradiance.py

---
 pvlib/tests/test_irradiance.py | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/pvlib/tests/test_irradiance.py b/pvlib/tests/test_irradiance.py
index 96effb2ab6..b5b65065ea 100644
--- a/pvlib/tests/test_irradiance.py
+++ b/pvlib/tests/test_irradiance.py
@@ -829,10 +829,7 @@ def test_ghi_from_poa_driesse():
     # 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=re.escape("xtol too small (%g <= 0)" % xtol)
-    ):
+    with pytest.raises(ValueError, match="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)

From 1f1cab713654845d4d95f1e13a86e8bd1d39265c Mon Sep 17 00:00:00 2001
From: echedey-ls <80125792+echedey-ls@users.noreply.github.com>
Date: Sat, 10 Feb 2024 23:36:07 +0100
Subject: [PATCH 05/10] Update v0.10.4.rst

---
 docs/sphinx/source/whatsnew/v0.10.4.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/sphinx/source/whatsnew/v0.10.4.rst b/docs/sphinx/source/whatsnew/v0.10.4.rst
index 3cab3fc8ad..5c529f6383 100644
--- a/docs/sphinx/source/whatsnew/v0.10.4.rst
+++ b/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
 ~~~~~~~

From 0433b9cdbe480ec54a5d30395f0a703faa6b8502 Mon Sep 17 00:00:00 2001
From: echedey-ls <80125792+echedey-ls@users.noreply.github.com>
Date: Sat, 10 Feb 2024 23:37:25 +0100
Subject: [PATCH 06/10] Am I faster that the linter?

---
 pvlib/tests/test_irradiance.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/pvlib/tests/test_irradiance.py b/pvlib/tests/test_irradiance.py
index b5b65065ea..7997ed1c2d 100644
--- a/pvlib/tests/test_irradiance.py
+++ b/pvlib/tests/test_irradiance.py
@@ -1,7 +1,6 @@
 import datetime
 from collections import OrderedDict
 import warnings
-import re
 
 import numpy as np
 from numpy import array, nan

From cc32be77b44bc7d455d75ff3141b3d97e22f045c Mon Sep 17 00:00:00 2001
From: echedey-ls <80125792+echedey-ls@users.noreply.github.com>
Date: Sat, 10 Feb 2024 23:46:38 +0100
Subject: [PATCH 07/10] Again the linter

---
 pvlib/tests/test_irradiance.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pvlib/tests/test_irradiance.py b/pvlib/tests/test_irradiance.py
index 7997ed1c2d..2ed54b0ca6 100644
--- a/pvlib/tests/test_irradiance.py
+++ b/pvlib/tests/test_irradiance.py
@@ -828,7 +828,7 @@ def test_ghi_from_poa_driesse():
     # 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="xtol too small \(%g <= 0\)" % xtol):
+    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)

From 176ce6574d73744c7699fc6702648019044f6f0a Mon Sep 17 00:00:00 2001
From: echedey-ls <80125792+echedey-ls@users.noreply.github.com>
Date: Sun, 11 Feb 2024 01:16:21 +0100
Subject: [PATCH 08/10] Clear up comments

---
 pvlib/irradiance.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/pvlib/irradiance.py b/pvlib/irradiance.py
index 7dfd356d04..6ecf5d4e4f 100644
--- a/pvlib/irradiance.py
+++ b/pvlib/irradiance.py
@@ -1514,12 +1514,13 @@ def poa_error(ghi):
                         disp=False,
                         )
     except ValueError as e:
-        # this may occur because poa_error has the same sign at both end points
         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]
@@ -1560,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]
     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).

From 2335eb23bb20244633dccb048030266d4c3bf38c Mon Sep 17 00:00:00 2001
From: echedey-ls <80125792+echedey-ls@users.noreply.github.com>
Date: Sun, 11 Feb 2024 22:09:10 +0100
Subject: [PATCH 09/10] Update irradiance.py

Co-Authored-By: Anton Driesse <9001027+adriesse@users.noreply.github.com>
---
 pvlib/irradiance.py | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/pvlib/irradiance.py b/pvlib/irradiance.py
index 6ecf5d4e4f..38b683585c 100644
--- a/pvlib/irradiance.py
+++ b/pvlib/irradiance.py
@@ -1513,15 +1513,11 @@ def poa_error(ghi):
                         full_output=True,
                         disp=False,
                         )
-    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
+    except ValueError:
+        # this occurs when poa_error has the same sign at both end points
+        ghi = np.nan
+        conv = False
+        niter = -1
     else:
         ghi = result[0]
         conv = result[1].converged
@@ -1597,6 +1593,9 @@ 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,

From 0fd74da550a8af7ec9c6016c2a9184bfa550a18e Mon Sep 17 00:00:00 2001
From: echedey-ls <80125792+echedey-ls@users.noreply.github.com>
Date: Sun, 11 Feb 2024 22:32:36 +0100
Subject: [PATCH 10/10] Update test_irradiance.py

---
 pvlib/tests/test_irradiance.py | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/pvlib/tests/test_irradiance.py b/pvlib/tests/test_irradiance.py
index 2ed54b0ca6..8b5991c7e5 100644
--- a/pvlib/tests/test_irradiance.py
+++ b/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,13 +825,21 @@ def test_ghi_from_poa_driesse():
     expected = [0, -1, 0]
     assert_allclose(expected, niter)
 
-    # test xtol propagation by producing an exception
+    # 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=r"xtol too small \(%g <= 0\)" % xtol):
+    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():