BUG: WIP: inconsistent broadcasting of outputs from singlediode methods in #409 only

[mikofski]

The issue was raised in some comments in #410 and #409 that the rules for broadcasting outputs are inconsistent among the single diode methods

• singlediode will only broadcast the output if photocurrent is an sequence, array or series
  • float -> OrderedDict of float
  • array or sequence -> OrderedDict of array or sequence
  • Series -> DataFrame
• i_from_v will broadcast the smallest size of either voltage, photocurrent, or resistance_shunt
• v_from_i will broadcast the smallest size of either current, photocurrent, or resistance_shunt

In those same comments @cwhanse proposed:

to expand singletons when any parameter input is a vector, and to fail (gracefully) if two parameters are vectors of unequal length

I agree with this idea, and both i_from_v and v_from_i already have some boilerplate code to address this (note this snippet is lifted from v_from_i, with variable names suited for that function, change the
arguments to other methods like singlediode or mpp):

    # find the right size and shape for returns
    args = (current, photocurrent, saturation_current,
            resistance_series, resistance_shunt, nNsVth)  <-- list the args allowed to broadcast here
    size, shape = 0, None  # 0 or None both mean scalar
    for arg in args:
        try:
            this_shape = arg.shape  # try to get shape
        except AttributeError:
            this_shape = None
            try:
                this_size = len(arg)  # try to get the size
            except TypeError:
                this_size = 0
        else:
            this_size = sum(this_shape)  # calc size from shape
            if shape is None:
                shape = this_shape  # set the shape if None
        # update size and shape
        if this_size > size:
            size = this_size
            if this_shape is not None:
                shape = this_shape

Now knowing the size and/or the shape one can let numpy.vectorize cast the output as proposed:

    if size <= 1:
        V = v_from_i_fun(*args)
        if shape is not None:
            V = np.tile(V, shape)  <-- hacky way to make either array(V), array([V]), array([[V]]), ...
    else:
        # np.vectorize handles broadcasting, raises ValueError
        vecfun = np.vectorize(v_from_i_fun)
        V = vecfun(*args)
    if np.isnan(V).any() and size <= 1:
        V = np.repeat(V, size)
        if shape is not None:
            V = V.reshape(shape)

On a related note: @cwhanse I realize now I misread your comment and made i_from_v and v_from_i fail silently instead of gracefully, oops!. This might cause some confusion, in the short term, but maybe we can address it in this issue here?

@wholmgren is there a PVLibException class? What exception would you expect here, maybe ValueError('All arguments must be either scalar or the same size.')?

[cwhanse]

I would prefer to fail verbosely and see an error message when the code for finding output shape etc. is presented with incompatible inputs. The code is going to fail at some point but it's a challenge for many users to walk back though the stack dump and see what they did wrong.

[mikofski]

✔️ addressed in f976f61 - fails verbosely if can't broadcast all inputs for i_from_v or v_from_i

[markcampanelli]

I'm curious if numpy.broadcast_arrays() gives errors that make sense to a user when the inputs don't jibe well together?

[mikofski]

IMO the error it raises is terse, but not opaque:

ValueError: operands could not be broadcast together with shapes (3,) () () (2,) () ()

[markcampanelli]

Oh Python, how I spend too much of my life sorting out your dynamic types!

[wholmgren]

I don't fully understand this issue.

singlediode returns multiple kinds of data. The pvlib style is to return labeled data in these situations. That means a DataFrame when the returned data is 2D and the input is a Series or DataFrame; OrderedDict if not. singlediode handles at least some kinds of numpy 2D+ inputs that I've used just fine. So, my experience with singlediode is that it already handles broadcasting for reasonable inputs. Maybe someone posted specific example of where it didn't do what you expected and I missed it in the flurry of discussion. Either way, could you (re)post it here?

i_from_v and v_from_i return one kind of data each, so they don't need the labeling machinery. I am surprised to read that they broadcast to the smallest of voltage, photocurrent, or resistance_shunt. That sounds like a bug. Perhaps that's due to something about np.broadcast_arrays that I don't understand (I don't have any experience with this function). Again, reproducible examples in this issue would help me to keep track of the status.

[mikofski]

Sorry @wholmgren I think this is specific to #409 proposed methods that didn't seem to be broadcasting correctly?

I've tested the existing methods in the master branch with different size inputs and they seem to broadcast okay, as long as numpy arrays are used. There does seem to be some preferential treatment for photocurrent as it can be a list or an array.

Here are some examples:

singlediode

• list of photocurrents, all other inputs scalar - works!
  singlediode([6, 7, 8], 1e-7, 0.001, 10, 1.23*0.026*72)
• same size arrays of photocurrents and shunt resistance, others scalar - works!
  singlediode(np.array([6, 7, 8]), 1e-7, 0.001, np.array([10, 11, 12]), 2.3)
• array of shunt resistance, all others scalar - works!
  singlediode(6, 1e-7, 0.001, np.array([10, 11, 12]), 2.3)
• same size arrays of shunt and series resistance, others scalar - works
  singlediode(6, 1e-7, np.array([0.001, 0.002, 0.003]), np.array([10, 11, 12]), 2.3)

i_from_v

• list of photocurrents, all other inputs scalar - works!
  i_from_v(10, 0.001, 2.3, 0.5, 1e-7, [6, 7, 8])
• same size list of voltages and photocurrents, others scalar - works!
  i_from_v(10, 0.001, 2.3, [0.4, 0.45, 0.5], 1e-7, [6, 7, 8])
• list of voltages, others scalar - works!
  i_from_v(10, 0.001, 2.3, [0.4, 0.45, 0.5], 1e-7, 6)
• same size array of Rsh and voltages, others scalar - works!
  i_from_v(np.array([10, 11, 12]), 0.001, 2.3, np.array([0.4, 0.45, 0.5]), 1e-7, 6)

I've change the title to make it specific to PR #409

[markcampanelli]

I did add various combinations like these to the v_from_i() and i_from_v() unit tests when I added ideal device support. It’s certainly not exhaustive though.

[cwhanse]

@mikofski can you confirm that this issue is closed by these lines: for Bishop88 with newton; here and here for LambertW methods.

Given how long this has been open and the absence of other complaints about broadcasting, I'm inclined to just close this.