Open cwhanse opened 5 years ago
Hi Cliff, I'm in support of making these changes. I also think #39 is relevant. ICYMI: @chetan201 and @cedricleroy
Hi @cwhanse @mikofski I think that's a great idea and I have considered it many times for variety of reasons. The only issue would be to make the changes in a way that retain the existing 2 diode model as-is for legacy compatibility. A lot of users (including us at SunPower) have tools based on the current implementation and it'd be too disruptive to revisit them.
Do you already have a WIP fork on a pvlib 1-diode model based implementation of PVMismatch?
retain the existing 2 diode model as-is
My suggestion in #39 was to keep the same (or similar) interface, but make pvcell
an argument that defaults to pvlib python singlediode
, but any class with the same (or newly specified) interface could also be used, For example, the existing 2-diode model or the DeSoto or CEC or 6-parameter model could be used.
One problem that @bmeyers found when he started working on #39 was that pvlib python doesn't have an expression for reverse bias. PVMismatch currently assumes a modified version of avalanche breakdown, but I think this should also be abstracted, and opened up to alternate expressions, such as a Zener breakdown model
Hi @cwhanse @mikofski I think that's a great idea and I have considered it many times for variety of reasons. The only issue would be to make the changes in a way that retain the existing 2 diode model as-is for legacy compatibility. A lot of users (including us at SunPower) have tools based on the current implementation and it'd be too disruptive to revisit them.
Got it.
Do you already have a WIP fork on a pvlib 1-diode model based implementation of PVMismatch?
I have a fork which has no work in it (yet).
One problem that @bmeyers found when he started working on #39 was that pvlib python doesn't have an expression for reverse bias. PVMismatch currently assumes a modified version of avalanche breakdown, but I think this should also be abstracted, and opened up to alternate expressions, such as a Zener breakdown model
@mikofski are you thinking to separate the IV curve calculation into two functions: forward and reverse bias?
separate the IV curve calculation into two functions: forward and reverse bias?
No, I think it's best to include the reverse bias in the same equation as the forward - perhaps we could do this in pvlib.singlediode.bishop88
the same way we incorporated the PVsyst-thin-film-recombination-current.
There equations are in the docstring in the example below, and represent the 2nd order avalanche breakdown used in PVMismatch. It is basically appended to the shunt current.
Equation 1: if b_rbd != 0
Equation 2: if b_rbd == 0
For reverse bias, we can do something like this:
"""
reverse bias breakdown (avalanche)
.. math::
V^\star = \frac{V_{diode}}{R_{sh}I_L}\\
I_{rbd} = \left(a_{rbd} V^\star + b_{rbd} \left(V^\star\right)^{2}\right)
I_L \left(1 - \frac{V_{diode}}{V_{rbd}} \right )^{-n_{rbd}}
.. note::
if :math:`b_{rbd}` is zero then this collapses to the following:
.. math::
I_{rbd} = \left(a_{rbd} \frac{V_{diode}}{R_{sh}}\right)
\left(1 - \frac{V_{diode}}{V_{rbd}} \right )^{-n_{rbd}}
v_rbd : float
reverse bias breakdown voltage, negative
a_rbd, b_rbd : float
avalanche breakdown 1st and 2nd order coefficients, positive, b may be zero
n_rbd : int
avalanche breakdown exponential parameter, positive, not zero
"""
v_star = v_diode * g_sh / photocurrent # normalized voltage
# current from avalanche breakdown in reverse bias
i_rbd = (a_rbd*v_star + b_rbd*v_star**2) * photocurrent * (1. - v_diode / v_rbd)**(-n_rbd)
# from pvlib.singlediode.bishop88
is_recomb = d2mutau > 0 # True where there is thin-film recombination loss
v_recomb = np.where(is_recomb, NsVbi - diode_voltage, np.inf)
i_recomb = np.where(is_recomb, photocurrent * d2mutau / v_recomb, 0)
# calculate temporary values to simplify calculations
v_star = diode_voltage / nNsVth # non-dimensional diode voltage
g_sh = 1.0 / resistance_shunt # conductance
i_diode = saturation_current * np.expm1(v_star) # current loss from recombination
i_shunt = diode_voltage * g_sh # current loss from shunting
# net cell current
i = photocurrent - i_diode - i_shunt - i_recomb - i_rbd
v = diode_voltage - i * resistance_series
I think we're saying the same thing: the currents are additive, so we can calculate i_rbd
and the other currents separately then add them. It may provide flexibility to implement the reverse bias current calculation in it's own function. The Bishop technique is quite useful here to avoid having to align voltages, since all equations for current can be expressed in terms of the diode voltage.
Proceeding in my fork:
PVCell
properties MODEL
one of 2diode
(default, current behavior), desoto
or pvsyst
desoto
and pvsyst
single diode modelsAll proceeding fine until the Voc method which uses the quadratic equation to solve for Voc (neglecting Rsh). This technique only works for the 2diode approach.
The docstring says "Estimate open circuit voltage". Is the value returned by Voc
updated somewhere else? I don't see that it happens in PVCell
. If the value here is the final value for Voc, for desoto
or pvsyst
models, we will need a function to calculate Voc, likely involving iteration.
Next issue: PVcell.Rsh
assumes a value that is constant over irradiance and temperature. For either desoto or pvsyst, the value depends on irradiance. An attribute is needed to contain the values calculated from Ee. If I maintain the current API, that attribute is Not PVcell.Rsh
.
I'd prefer to change PVcell.Rsh
to PVcell.Rsh_STC
which frees up PVcell.Rsh
to be a Property
. PVcell._Rsh
seems obtuse. Rsh_STC
would be accompanied by attributes Rsh_0
and Rsh_exp
for the pvsyst model.
My preference would break the current API for PVcell
. Thoughts?
yes, agreed, please break the api. I agree with your suggestion
I'd prefer to change
PVcell.Rsh
toPVcell.Rsh_STC
which frees upPVcell.Rsh
to be a Property.PVcell._Rsh
seems obtuse.Rsh_STC
would be accompanied by attributesRsh_0
andRsh_exp
for the pvsyst model.
This more or less follows the same pattern as for Isat2fix
in #64 I think. Basically replace Rsh
everywhere with Rsh0
and then create a new property Rsh
that implements the temperature dependent behavior. I think Isat1
and Isc0
also have similar patterns. Although in this case because Rsh_0
is also a property, then maybe Rsh_STC
is more meaningful. I'll leave that up to you and @chetan201 .
Sorry I haven't been more proactive on this. Thank you so much for driving this forward. I wish I could contribute more.
Hi Cliff,
The estimate Voc method is similar to the estimate_voc
in pvlib.singlediode
. They follow the same assumption, let Rsh -> Inf
and Rs -> 0
.
You're right aboutVoc
it only gets calculated for the system in pvsystem.calcMPP_IscVocFFeff()
. It didn't seem necessary to get an accurate value of Voc
since just an approximation is good enough for bounding the MPP and spacing out the IV curve points sufficiently.
We can add a property, Voc
, that interpolates the voltage where current is zero, and rename the estimate to voc_est
Also at some point we should make this code BLACK
@cwhanse and @mikofski I think we are ok with breaking the API to introduce Rsh Irradiance dependance. Is there a way to keep it backward compatible? (I know I didn't do it for Isat2 fix! :/ ) If not, let's warn the user for these major API changes so that they don't waste time figuring out why the legacy code doesn't work with new stuff. Thanks!
@cwhanse and @mikofski I think we are ok with breaking the API to introduce Rsh Irradiance dependance. Is there a way to keep it backward compatible?
The current 2-diode model is still available, with changes to some attribute and property names. In this sense it's backward compatible.
If by warn you mean issue a runtime warning if, for example, a user sets or references the old Rsh
attribute, I don't know how that would work. The better approach (if all these changes are accepted) would be to bump to v5.0 and tell users in release notes.
But if that's the way ahead, I recommend considering an overhaul of the whole API for the class layer. Using the named parameters as attributes with setters presupposes the diode model and is a bit awkward to extend. Worth considering the more general approach @mikofski suggests : a class with an attribute that points to a function which translates irradiance and temperature to the 5 (7) parameters for the one (two) diode equation. The downside is losing control over the naming of the parameters for the diode model.
Opening a very general issue to discuss work that could be relevant to this project.
I have a need to compute IV curves taking into account various external influences (all modeled): soiling, shading, interconnect resistance, cell-to-cell variations e.g. cracking, and others. To accomplish this it appears I would need to make substantial changes and additions. Some of the external influences can be handled through a cell-by-cell map of effective irradiance; for others, it appears that PVMismatch would need some substantial work (to specify a value of series resistance by cell, for example).
I'd prefer to use a single diode equivalent circuit with one of the various sets of equations for model parameter dependence on irradiance and temperature. #88 is relevant, as is #75, #74, #57, other issues. I'm fairly sure I want that model to use photocurrent as input rather than Isc as expected by
pvcell.Aph
.How much of the above is of interest for PVMismatch's core? I'm OK if the answer is "not much but thanks anyway". I am reluctant to build on PVMismatch if major changes would only live in my fork.