Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement one-diode models #115

Open
cwhanse opened this issue Oct 29, 2019 · 15 comments
Open

Implement one-diode models #115

cwhanse opened this issue Oct 29, 2019 · 15 comments

Comments

@cwhanse
Copy link

cwhanse commented Oct 29, 2019

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.

@mikofski
Copy link
Contributor

Hi Cliff, I'm in support of making these changes. I also think #39 is relevant.
ICYMI: @chetan201 and @cedricleroy

@chetan201
Copy link
Contributor

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?

@mikofski
Copy link
Contributor

mikofski commented Nov 2, 2019

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.

@mikofski
Copy link
Contributor

mikofski commented Nov 2, 2019

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

@cwhanse
Copy link
Author

cwhanse commented Nov 4, 2019

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).

@cwhanse
Copy link
Author

cwhanse commented Nov 4, 2019

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?

@mikofski
Copy link
Contributor

mikofski commented Nov 4, 2019

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
equation1

Equation 2: if b_rbd == 0
equation2

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

@cwhanse
Copy link
Author

cwhanse commented Nov 4, 2019

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.

@cwhanse
Copy link
Author

cwhanse commented Nov 27, 2019

Proceeding in my fork:

  • added PVCell properties MODEL one of 2diode (default, current behavior), desoto or pvsyst
  • added N1 and N2 for the ideality factors (default to 1.0 and 2.0, respectively). Adding N1 opens possibility for desoto and pvsyst single diode models
  • added MU_GAMMA (temp. co. for N1 in pvsyst model) and DEG_DT (temp. co. for Eg in desoto model)

All 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.

@cwhanse cwhanse changed the title Development plan for PVMismatch? Implement one-diode models Nov 27, 2019
@cwhanse
Copy link
Author

cwhanse commented Nov 27, 2019

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?

@mikofski
Copy link
Contributor

mikofski commented Nov 28, 2019

yes, agreed, please break the api. I agree with your suggestion

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.

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.

@mikofski
Copy link
Contributor

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

@chetan201
Copy link
Contributor

@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!

@mikofski
Copy link
Contributor

mikofski commented Dec 13, 2019

Another approach that would not break compatibility is to create an abstract class that implements the same interface, then any model could supplied. But this would require changes to PVmodule, PVstring, and PVsystem for example setSuns, calcMod so I don't know if it's possible.

@cwhanse
Copy link
Author

cwhanse commented Dec 13, 2019

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants