Revise golden mean search convergence (#1606)

* revise convergence, add test with equal upper and lower

* whatsnew

* clarify error message

* improve from review

* length 1 array test

* remove try/except not needed

Author: cwhanse

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

@@ -12,19 +12,21 @@ Enhancements
 * Multiple code style issues fixed that were reported by LGTM analysis. (:issue:`1275`, :pull:`1559`)
 * Added a direct IAM model :py:func:`pvlib.iam.schlick` which can be used with
   :py:func:`~pvlib.iam.marion_diffuse`, and a diffuse IAM model
-  :py:func:`pvlib.iam.schlick_diffuse` (:pull:`1562`, :issue:`1564`)
+  :py:func:`pvlib.iam.schlick_diffuse`. (:pull:`1562`, :issue:`1564`)
 * Added a function to calculate one of GHI, DHI, and DNI from values of the other two.
-  :py:func:`~pvlib.irradiance.complete_irradiance`
+  :py:func:`~pvlib.irradiance.complete_irradiance`.
   (:issue:`1565`, :pull:`1567`)
 * Add optional ``return_components`` parameter to :py:func:`pvlib.irradiance.haydavies` to return
-  individual diffuse irradiance components (:issue:`1553`, :pull:`1568`)
+  individual diffuse irradiance components. (:issue:`1553`, :pull:`1568`)
 
 
 Bug fixes
 ~~~~~~~~~
 
 * Fixed bug in :py:func:`pvlib.shading.masking_angle` and :py:func:`pvlib.bifacial.infinite_sheds._ground_angle`
-  where zero ``gcr`` input caused a ZeroDivisionError (:issue:`1576`, :pull:`1589`)
+  where zero ``gcr`` input caused a ZeroDivisionError. (:issue:`1576`, :pull:`1589`)
+* Fixed bug in :py:func:`pvlib.tools._golden_sect_DataFrame` so that a result is returned when the search
+  interval is length 0 (which occurs in :py:func:`pvlib.pvsystem.singlediode` if v_oc is 0.) (:issue:`1603`, :pull:`1606`)
 
 Testing
 ~~~~~~~

pvlib/tests/test_tools.py

@@ -45,6 +45,22 @@ def test__golden_sect_DataFrame_vector():
     v, x = tools._golden_sect_DataFrame(params, lower, upper,
                                         _obj_test_golden_sect)
     assert np.allclose(x, expected, atol=1e-8)
+    # some upper and lower bounds equal
+    params = {'c': np.array([1., 2., 1.]), 'n': np.array([1., 1., 1.])}
+    lower = np.array([0., 0.001, 1.])
+    upper = np.array([1., 1.2, 1.])
+    expected = np.array([0.5, 0.25, 1.0])  # x values for maxima
+    v, x = tools._golden_sect_DataFrame(params, lower, upper,
+                                        _obj_test_golden_sect)
+    assert np.allclose(x, expected, atol=1e-8)
+    # all upper and lower bounds equal, arrays of length 1
+    params = {'c': np.array([1.]), 'n': np.array([1.])}
+    lower = np.array([1.])
+    upper = np.array([1.])
+    expected = np.array([1.])  # x values for maxima
+    v, x = tools._golden_sect_DataFrame(params, lower, upper,
+                                        _obj_test_golden_sect)
+    assert np.allclose(x, expected, atol=1e-8)
 
 
 def test__golden_sect_DataFrame_nans():

pvlib/tools.py

@@ -341,6 +341,8 @@ def _golden_sect_DataFrame(params, lower, upper, func, atol=1e-8):
     --------
     pvlib.singlediode._pwr_optfcn
     """
+    if np.any(upper - lower < 0.):
+        raise ValueError('upper >= lower is required')
 
     phim1 = (np.sqrt(5) - 1) / 2
 
@@ -349,16 +351,8 @@ def _golden_sect_DataFrame(params, lower, upper, func, atol=1e-8):
     df['VL'] = lower
 
     converged = False
-    iterations = 0
 
-    # handle all NaN case gracefully
-    with warnings.catch_warnings():
-        warnings.filterwarnings(action='ignore',
-                                message='All-NaN slice encountered')
-        iterlimit = 1 + np.nanmax(
-            np.trunc(np.log(atol / (df['VH'] - df['VL'])) / np.log(phim1)))
-
-    while not converged and (iterations <= iterlimit):
+    while not converged:
 
         phi = phim1 * (df['VH'] - df['VL'])
         df['V1'] = df['VL'] + phi
@@ -373,22 +367,16 @@ def _golden_sect_DataFrame(params, lower, upper, func, atol=1e-8):
 
         err = abs(df['V2'] - df['V1'])
 
-        # works with single value because err is np.float64
-        converged = (err[~np.isnan(err)] < atol).all()
-        # err will be less than atol before iterations hit the limit
-        # but just to be safe
-        iterations += 1
-
-    if iterations > iterlimit:
-        raise Exception("Iterations exceeded maximum. Check that func",
-                        " is not NaN in (lower, upper)")  # pragma: no cover
+        # handle all NaN case gracefully
+        with warnings.catch_warnings():
+            warnings.filterwarnings(action='ignore',
+                                    message='All-NaN slice encountered')
+            converged = np.all(err[~np.isnan(err)] < atol)
 
-    try:
-        func_result = func(df, 'V1')
-        x = np.where(np.isnan(func_result), np.nan, df['V1'])
-    except KeyError:
-        func_result = np.full_like(upper, np.nan)
-        x = func_result.copy()
+    # best estimate of location of maximum
+    df['max'] = 0.5 * (df['V1'] + df['V2'])
+    func_result = func(df, 'max')
+    x = np.where(np.isnan(func_result), np.nan, df['max'])
 
     return func_result, x