pvlib / pvlib-python

A set of documented functions for simulating the performance of photovoltaic energy systems.
https://pvlib-python.readthedocs.io
BSD 3-Clause "New" or "Revised" License
1.16k stars 980 forks source link

Single diode model helper function to wrap Lambert w and other methods #410

Closed mikofski closed 6 years ago

mikofski commented 6 years ago

I propose the following:

markcampanelli commented 6 years ago

It appears that singlediode() does not expose a way to compute i-from-v at an arbitrary array of v. One would use i_from_v() for that. Also, singlediode() makes a choice for i_from_v(), v_from_i(), and the maximum power computation.

Also, I have vectorized max power and fill factor functions that I think can be very useful for plotting max power and ff as a function of temperature and/or photocurrent, for example.

I think this will first and foremost require some careful thinking about what the current API definition is and what we want it to be going forward with alternative implementations.

cwhanse commented 6 years ago

Guys this is great, by all means press on. When I can get back to my desk I'll contribute, meanwhile I'll just follow the fun from my phone.

Sent from my iPhone

On Jan 29, 2018, at 4:49 PM, Mark Campanelli notifications@github.com<mailto:notifications@github.com> wrote:

It appears that singlediode() does not expose a way to compute i-from-v at an arbitrary array of v. One would use i_from_v() for that. Also, singlediode() makes a choice for i_from_v(), v_from_i(), and the maximum power computation.

Also, I have vectorized max power and fill factor functions that I think can be very useful for plotting max power and ff as a function of temperature and/or photocurrent, for example.

I think this will first and foremost require some careful thinking about what the current API definition is and what we want it to be going forward with alternative implementations.

- You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/pvlib/pvlib-python/issues/410#issuecomment-361288223, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AFJNLzHXrvN9cs_hGI88KWWjylUT5Nfbks5tPeh-gaJpZM4RwCs-.

wholmgren commented 6 years ago

Makes sense to me. We can do this without any breaking API changes, right?

mikofski commented 6 years ago

Yes, that's my view - the api will remain the same. Is it okay to make bishop88 and it's related i_from_v, v_from_i, etc. methods the defaults?

wholmgren commented 6 years ago

I need to review the paper and probably see a clean PR before I can recommend a default.

markcampanelli commented 6 years ago

I'd really like to know where pandas support starts and ends in the API. I was told earlier that I didn't need to match output to pandas input in i_from_v() and v_from_i(). I realize folks love pandas, but it's hard enough just wrangling numpy types into some sort of consistency (esp. python scalar vs. numpy scalar vs. dim-0 array vs. dimensioned array)!

wholmgren commented 6 years ago

The rule of thumb is pandas in, pandas out. Beyond that, I'd just say don't remove pandas consistency from functions that do already support it. *_from_* does not, singlediode does.

I'm guessing that most users would consider pandas consistency to be more important than numpy type consistency.

markcampanelli commented 6 years ago

Thanks. I think the i_from_v() and v_from_i() docstrings indicate that pandas.Series are ok inputs, but won't be the output. I propose removing that promise on the inputs and leave it to higher level functions.

wholmgren commented 6 years ago

In general, it's ok with me to remove it as a promise on the inputs of a function if it's not also a promise on the outputs. In most cases I think it's much better to have a feature that only supports numpy than a feature that doesn't exist.

markcampanelli commented 6 years ago

@mikofski Note the above pandas support discussion for #409.

markcampanelli commented 6 years ago

@wholmgren Is numba used in pvlib-python yet? If not, then are there remaining blockers to using it?

wholmgren commented 6 years ago

@thunderfish24 yes, numba can be used in solar position calculations. See spa.py and get_solarposition(method='nrel_numba').

markcampanelli commented 6 years ago

Background: It occurs to me that the "bracketing problem" for brentq-style *_from_*() solvers could possibly be handled more effectively when there is an enitre I-V curve to compute, because of the strictly monotonic decreasing nature of I-V curves from the single-diode model.

I am unclear about all the different input-output combinations guaranteed by the API for the singlediode() function.

In particular, I am wondering if a user had, say, a 1000-point random sample from a distribution of all 5 parameters to the single-diode model, and the user wanted to compute 51-point I-V curves for each of these samples, then is singlediode() supposed to be able to calculate these in a "one shot/vectorized" manner (e.g., without putting it into a for loop)? If yes, then what exactly does output dictionary look like (e.g., the dimensions of the contents)?

Furthermore, the documentation says:

The output will be an OrderedDict if photocurrent is a scalar,
array, or ivcurve_pnts is not None.

The output will be a DataFrame if photocurrent is a Series and
ivcurve_pnts is None.

However, it seems that there are other combinations not covered here:

wholmgren commented 6 years ago

then is singlediode() supposed to be able to calculate these in a "one shot/vectorized" manner (e.g., without putting it into a for loop)?

Are you asking if singlediode can handle inputs of numpy arrays with dim > 1? The answer is yes. Output will be an OrderedDict of numpy arrays.

Why is photocurrent special?

It's the first argument. It's a pragmatic and simple approach. It worked for me.

What about if nNsVth is the only Series input?

What is the use case for this being a Series but photocurrent being an array or singleton?

What if photocurrent is a Series and ivcurve_pnts is not None?

OrderedDict of arrays.

cwhanse commented 6 years ago

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

For points, I'm ok with the default output being Voc, Isc and Pmp/Vmp/Imp. Personally I rarely find use for Ix and Ixx, preferring instead to compute the curve. We should offer npoints (can discuss how they are chosen) or accept a list of voltage or current, at which the corresponding value is returned. I'd return NaN for out of bounds values.

I think these output options cover the great majority of uses which are these three cases:

If there's another use case I'm overlooking please speak up.

Cliff

Sent from my iPhone

On Jan 31, 2018, at 8:48 AM, Will Holmgren notifications@github.com<mailto:notifications@github.com> wrote:

then is singlediode() supposed to be able to calculate these in a "one shot/vectorized" manner (e.g., without putting it into a for loop)?

Are you asking if singlediode can handle inputs of numpy arrays with dim > 1? The answer is yes. Output will be an OrderedDict of numpy arrays.

Why is photocurrent special?

It's the first argument. It's a pragmatic and simple approach. It worked for me.

What about if nNsVth is the only Series input?

What is the use case for this being a Series but photocurrent being an array or singleton?

What if photocurrent is a Series and ivcurve_pnts is not None?

OrderedDict of arrays.

- You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/pvlib/pvlib-python/issues/410#issuecomment-361974051, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AFJNL1Mqm7alpR5sMAT0dajaJS7IFc9xks5tQItOgaJpZM4RwCs-.

markcampanelli commented 6 years ago

I would have figured that I_x and I_xx would come in handy for comparison to the Sandia Model. I'd be fine leaving those in, even if just for backwards compatibility. It also seems like specifying a vector of voltages and/or currents should be able to be added optionally in a backwards compatible manner with ivcurve_pnts. NaN's sound ok to me too for values out of the first quadrant. Slopes at Voc and at Isc can be useful for measurement labs that often estimate these from local affine fits to I-V curve data as opposed to from a fit single diode model. It is also illustrative to compare how much these slopes differ from the series and shunt resistances.

Seems like Monte Carlo simulations would be better served by lower level functions than singlediode(), especially the *_from_* one(s) that aren't restricted to the first quadrant, because the I-V curves "jiggle" around as the sampled parameter sets vary.

mikofski commented 6 years ago

FYI bishop88(gradients=True) will return di/dv and dp/dv at the desired points, eg Isc or Voc , useful for example for checking the value of Rsh.

mikofski commented 6 years ago

I've implemented both i_from_v(method="gold"), v_from_i(method="gold"), and mppt(method="gold") so you can try mc if you want.

cwhanse commented 6 years ago

I have no objections to retaining Ix and Ixx. I'm in favor of adding the dynamic resistances at Isc and Voc, which we can compute using exact expressions rather than fitting to the modeled IV curve.

Can we rename mppt() to mpp()? The 't' is 'tracking' which isn't the functionality we're providing.

mikofski commented 6 years ago

✔️ changed pvlib.pvsystem.mppt() -> mpp(), pvlib.singlediode_methods.fast_mppt() -> fast_mpp(), and ditto for slow_mpp() in ff8cc0b

mikofski commented 6 years ago

✔️ the Ix and Ixx values are returned using any method: ('lambertw', 'fast', default: 'gold')

Exact expressions for gradients from bishop88(gradients=True)

✔️ both dynamic resistances: dI/dV @ (V, I) = (0, Isc), and dI/dV @ (V, I) = (Voc, 0) can both be evaluated using exact expressions from pvsystem.bishop88(gradients=True):

>>> from pvlib import pvsystem
>>> IL, I0, Rs, Rsh, nNsVth = (7, 6e-7, .1, 20, .5)
>>> out = singlediode(IL, I0, Rs, Rsh, nNsVth)
>>> vd_sc = out['i_sc']*Rs  # diode voltage at short circuit
>>> short_circuit = bishop88(vd_sc, IL, I0, Rs, Rsh, nNsVth, gradients=True)
>>> short_circuit
(6.965172322158323,        # i_sc,       idx=0
 -9.9920072216264089e-16,  # v_sc,       idx=1
 -6.9596052142878349e-15,  # p_sc,       idx=2
 -0.05000483246177085,     # di/dvd,     idx=3
 1.0050004832461772,       # dv/dvd,     idx=4
 -0.049756028275980489,    # di/dv,      idx=5
 6.965172322158323,        # dp/dv,      idx=6
 -0.1000096649235417)      # d2p/dv/dvd, idx=7
>>> grad_sc = short_circuit[5]
>>> -1 / grad_sc
20.098067202095102  # super close to Rsh = 20.

etc.

The important thing to note is that Bishop expected the diode voltage, vd, not the cell voltage, that's how it's explicit, but it's easy to convert between them, and I suppose we could make a conversion method if required:

def diode_voltage(i, v, rs):
    """calculate diode voltage"""
    return v + i * rs

def v_from_vd(vd, i, rs):
    """calculate cell voltage from diode voltage"""
    return vd - i * rs
markcampanelli commented 6 years ago

So #412 offers alternatives to working with vd. There is some groundwork to do before I would try to add them to pvlib (i.e., #411), but if they test and benchmark well, I will offer them up. Note that the idea in #412 for (1) brentq is to upper bound the 1st quadrant I-V curve with the corresponding ideal-device curve, and (2) newton/halley is to use a partially ideal device as the initial condition (all quadrants). I suspect that (2) may not converge globally, esp. for really poor quality devices. I'd also like to try out numba on both of these methods.

nit: I prefer p_mp and p_mp() for maximum power variable and function-that-computes maximum power names, respectively. Jibes better with i_sc and v_oc in my own mind.

mikofski commented 6 years ago

nit: I prefer p_mp and p_mp() for maximum power variable and function-that-computes maximum power names, respectively. Jibes better with i_sc and v_oc in my own mind.

Are you referring to mpp()? The output is already p_mp as you suggest. Personally, I don't think that the function that calculates an output and the output itself should have the same name, unless it's a property, but that's just my opinion. So I'm okay with it as it is. Is that okay with you?

I am starting to feel for sanity sake that we should probably start to think of a class to hold all of these single diode model methods and attributes, but let's save that for another day.

cwhanse commented 6 years ago

Agree with the class idea, but not now. I suspect it will break the current API.

cwhanse commented 6 years ago

@mikofski @thunderfish24 do you have any updates on this and related issues #409 #411? Will and I are campaigning to get pvlib to 0.6.0 and to clear up the backlog of issues and pull requests.

markcampanelli commented 6 years ago

I'm supposed to have a considerable chunk of free time on Saturday, so if all goes well I can address the reviews on PR #426 for issue #411 and it should then be ready to go in. IIRC, #411 has two additional pieces after #426: (1) Adding gold data for Isc, Ix, Pmp, Ixx, and Voc, and (2) Adding unit tests that actually fail if the pvsystem functions don't produce values sufficiently close to the gold values. (A further followup could add logging/tracking of the benchmarks in CI.) Of course, I see no reason that these additional items cannot happen after 0.6.0.

mikofski commented 6 years ago

@cwhanse I have been waiting for SciPy pr8357 to merge. It's almost there. The last reviewers remaining issue pr8907 / gh8904 was merged today, coincidentally, I can work on it tomorrow and then we should see vectorized Newton in SciPy-1.2.0 but when will that be released? Is it okay for us to use a vendorized version until SciPy is updated? Or should we do some Numba hack in the meantime? Or just add Bishop explicit method but leave everything else the same. Sorry, I don't have a good answer. I will try to wrap this up as soon as possible

wholmgren commented 6 years ago

I recommend copying the vectorized newton function into _tools.py for now.

mikofski commented 6 years ago

@wholmgren you mean tools.py no underscore right?

wholmgren commented 6 years ago

Yes. But I think the function should be private.

mikofski commented 6 years ago

OK if I move _golden_sect_DataFrame to tools.py also? no changes to API. makes it easier to not have circular references. thanks! You'll probably dislike this, but I'm calling that instead of brentq until I get https://github.com/scipy/scipy/pull/8431

wholmgren commented 6 years ago

OK if I move _golden_sect_DataFrame to tools.py also?

Sure.

You'll probably dislike this, but I'm calling that instead of brentq until I get scipy/scipy#8431

I am confused. What will I dislike? And I thought you were going to use your new vectorized newton for single diode.

mikofski commented 6 years ago

409 is long and convoluted. We agreed in the end to have 3 methods: