_golden_sect_DataFrame changes in 0.9.4

[cedricleroy]

Describe the bug

0.9.4 introduced the following changes in the _golden_sect_DataFrame: We are checking upper and lower parameters and raise an error if lower > upper.

pvlib-python/pvlib/tools.py

Lines 344 to 345 in 81598e4

	if np.any(upper - lower < 0.):
	raise ValueError('upper >= lower is required')

_golden_sect_DataFrame is used by _lambertw:

pvlib-python/pvlib/singlediode.py

Lines 644 to 649 in 81598e4

	# Compute open circuit voltage
	v_oc = _lambertw_v_from_i(0., **params)
	# Find the voltage, v_mp, where the power is maximized.
	# Start the golden section search at v_oc * 1.14
	p_mp, v_mp = _golden_sect_DataFrame(params, 0., v_oc * 1.14, _pwr_optfcn)

I often have slightly negative v_oc values (really close to 0) when running simulations (second number in the array below):

array([ 9.46949758e-16, -8.43546518e-15,  2.61042547e-15,  3.82769773e-15,
        1.01292315e-15,  4.81308106e+01,  5.12484772e+01,  5.22675087e+01,
        5.20708941e+01,  5.16481028e+01,  5.12364071e+01,  5.09209060e+01,
        5.09076598e+01,  5.10187680e+01,  5.11328118e+01,  5.13997628e+01,
        5.15121386e+01,  5.05621451e+01,  4.80488068e+01,  7.18224446e-15,
        1.21386700e-14,  6.40136698e-16,  4.36081007e-16,  6.51236255e-15])

If we have one negative number in a large timeseries, the simulation will crash which seems too strict.

Expected behavior

That would be great to either:

• Have this data check be less strict and allow for slightly negative numbers, which are not going to affect the quality of the results.
• On _lambertw: Do not allow negative v_oc and set negative values to np.nan, so that the error is not triggered. It will be up to the upstream code (user) to manage those np.nan.

Versions:

• pvlib.__version__: >= 0.9.4
• pandas.__version__: 1.5.3
• python: 3.10.11

[echedey-ls]

See #1673

[cwhanse]

@cedricleroy can you provide the inputs and function call that produced the negative v_oc shown above?

[cedricleroy]

@echedey-ls Thanks! I thought I checked for related issues, but apparently not enough 😄

@cwhanse Sure thing:

Running _lambertw_v_from_i in _lambertw with the following data:

    resistance_shunt  resistance_series    nNsVth  current  saturation_current  photocurrent          v_oc
0        8000.000000              0.178  1.797559      0.0        1.480501e-11      0.000000  8.306577e-16
1        8000.000000              0.178  1.797048      0.0        1.456894e-11      0.000000 -7.399531e-15
2        8000.000000              0.178  1.791427      0.0        1.220053e-11      0.000000  2.289847e-15
3        8000.000000              0.178  1.789892      0.0        1.162201e-11      0.000000  3.357630e-15
4        8000.000000              0.178  1.790915      0.0        1.200467e-11      0.000000  8.885291e-16
5        7384.475098              0.178  1.796786      0.0        1.444902e-11      0.237291  4.222001e+01
6        5023.829590              0.178  1.814643      0.0        2.524836e-11      1.458354  4.495480e+01
7        2817.370605              0.178  1.841772      0.0        5.803733e-11      3.774055  4.584869e+01
8        1943.591919              0.178  1.877364      0.0        1.682954e-10      6.225446  4.567622e+01
9        1609.391479              0.178  1.910984      0.0        4.479085e-10      8.887444  4.530535e+01
10       1504.273193              0.178  1.937034      0.0        9.402419e-10     11.248103  4.494422e+01
11       1482.143799              0.178  1.951216      0.0        1.399556e-09     12.272360  4.466746e+01
12       1485.013794              0.178  1.950762      0.0        1.381967e-09     12.114989  4.465584e+01
13       1506.648315              0.178  1.942643      0.0        1.100982e-09     11.167084  4.475331e+01
14       1580.780029              0.178  1.928508      0.0        7.387948e-10      9.350249  4.485334e+01
15       1832.828735              0.178  1.901971      0.0        3.453772e-10      6.842797  4.508751e+01
16       2604.075684              0.178  1.869294      0.0        1.325485e-10      4.191604  4.518609e+01
17       4594.301270              0.178  1.844949      0.0        6.390201e-11      1.771347  4.435276e+01
18       6976.270996              0.178  1.829467      0.0        3.987927e-11      0.409881  4.214808e+01
19       8000.000000              0.178  1.821491      0.0        3.120619e-11      0.000000  6.300214e-15
20       8000.000000              0.178  1.813868      0.0        2.464867e-11      0.000000  1.064796e-14
21       8000.000000              0.178  1.809796      0.0        2.171752e-11      0.000000  5.615234e-16
22       8000.000000              0.178  1.808778      0.0        2.103918e-11      0.000000  3.825272e-16
23       8000.000000              0.178  1.806231      0.0        1.943143e-11      0.000000  5.712599e-15

data.csv

[cwhanse]

If we have one negative number in a large timeseries, the simulation will crash which seems too strict.

Agree this is not desirable.

My thoughts:

1. We could insert v_oc = np.maximum(v_oc, 0) above this line. That would preserve nan.
2. I am reluctant to change _lambertw_v_from_i. That function's job is to solve the diode equation, which is valid for negative current. I don't think this function should make decisions about its solution. There will always be some degree of imprecision (currently it's around 10-13 or smaller, I think).
3. I am also reluctant to change _golden_sect_DataFrame for similar reasons - the function's job should be to find a minimum using the golden section search. Complying with the lower < upper requirement is the job of the code that calls this function.

[cedricleroy]

1/ makes sense to me. I agree with the CONS for 2/ and 3/

Happy to open a PR with 1. if that helps.

[cwhanse]

Happy to open a PR with 1. if that helps.

That is welcome. Because I'm cautious about junk values with larger magnitude being covered up by 0s, maybe

v_oc[(v_oc < 0) & (v_oc > 1e-12)] = 0.