Behavior of calcparams and singlediode function could be improved

[cwhanse]

The type and shape of output from calcparams and singlediode functions have some oddities that make the output somewhat annoying and may pose a risk of code breaking if pandas/numpy broadcasting and/or typecasting change.

Observations:

• When pvsystem.calcparams_ functions have float inputs, some of the outputs are arrays.
• When pvsystem.calcparams_ functions have Series input, not all output are Series, and some of the output Series inherits a name from the input Series.
• When pvsystem.singlediode has length 1 irradiance and temperature, and ivcurve_pnts=N is specified, the returned arrays are shape (1, N), and so must be transposed to plot the I-V curve.
• When ivcurve_pnts=None (default) pvsystem.singlediode returns a DataFrame. When ivcurve_pnts is specified, pvsystem.singlediodereturns an OrderedDict.
• When multiple IV curves are to be calculated with IV curve points, the IV curves are row-by-row, so must be transposed to be plotted.

To Reproduce
gist here

Expected behavior

• calcparams functions return floats when all inputs are floats
• when Series are returned, don't inherit name from input (e.g. 'temp_cell' for the saturation current)
• IV curve points should be transposed for convenient plotting
• Consider always returning a DataFrame from singlediode.

Versions:

• pvlib.__version__: 0.9.4
• pandas.__version__: 1.2.4

[wholmgren]

Nice gist.

When pvsystem.calcparams_ functions have Series input, not all output are Series,

Do you propose casting resistance_series to a series of same length as the input?

and some of the output Series inherits a name from the input Series.

This problem is not unique to these functions. A couple of functions have code like this, but I would hate to see this in every function:

pvlib-python/pvlib/irradiance.py

Lines 225 to 228 in 57625c3

	try:
	aoi_value.name = 'aoi'
	except AttributeError:
	pass

A wrapper function for input/output types could solve this among other problems. Also item 1.

When ivcurve_pnts=None (default) pvsystem.singlediode returns a DataFrame. When ivcurve_pnts is specified, pvsystem.singlediodereturns an OrderedDict.

Related discussion in #418. I still think ivcurve_pnts should be removed from singlediode. That could be a good path to solving items 3-5.

[cwhanse]

My primary aims would be to 1) have these functions return consistent output for single vs. array-like input, and 2) avoid .T when making plots.

Yes, I think resistance_series should be returned from calcparams in the same type as other outputs.

I'd be OK if singlediode (and i_from_v, v_from_i) always returned DataFrame.

I'm willing to agree to removing ivcurves_pnts because we have pvsystem.i_from_v and .v_from_i functions (and methods).

[cwhanse]

Here's another corner case that we missed with tests. The vectorized newton solver doesn't work if parameters are Series of length one.

import pandas as pd
import pvlib


args = (0.001, 1.5, 6., 5e-9, 1000., 0.5)
params = pvlib.pvsystem.calcparams_desoto(1000., 25, *args)
params_series = pvlib.pvsystem.calcparams_desoto(pd.Series(data=[1000.]),
                                                 pd.Series([25.]), *args)
params_series2 = pvlib.pvsystem.calcparams_desoto(pd.Series(data=[1000., 1000.]),
                                                  pd.Series([25., 25.]), *args)
# works with each input as float
result = pvlib.pvsystem.singlediode(*params, method='newton')

# works with Series if length > 1
result_series2 = pvlib.pvsystem.singlediode(*params_series2, method='newton')

# errors with Series if length is 1
result_series = pvlib.pvsystem.singlediode(*params_series, method='newton')

[adriesse]

Good ideas and initiatives. Regarding the transposition for convenient graphing, I have the impression I have to do this from time to time with results from numpy or scipy too. Most important thing is that the output shapes broadcast appropriately to downstream vectorized calculations.

[kandersolar]

@cwhanse @reepoi with #1700 and #1743 both merged, should this issue be closed?

[reepoi]

Those two pull requests completed the checkmarks in the opening comment of this issue, so I think it can be closed.
They did not address the bug brought up in this comment, which is an issue with pvlib.singlediode.bishop88_v_from_i. I think a new issue should be created for that.

[cwhanse]

Agree, we can close. I made #1787 for #1626 (comment)

[cwhanse]

Closed via #1700 and #1743