Add De Soto, PVsyst single diode models

[cwhanse]

Summary of API breaks:

• PVcell.Rsh is replaced by PVcell.Rsh_STC. PVcell.Rsh now holds condition-dependent Rsh.
• PVcell.Eg is replaced by PVcell.Eg_0. PVcell.Eg now holds condition-dependent Eg.
• Added PVcell.N1_0 and PVcell.N2_0 to contain diode (ideality) factors, which are used in place of the previously hard-coded 1.0 and 2.0 values.

Not yet done: PVcell.Voc still uses diode factors 1.0 and 2.0 for the 2-diode model.

Opening PR to get some feedback on the changes. No tests included.

[mikofski]

Hi @cwhanse, I have a few comments, feel free to ignore:

1. Something similar to your proposal for shunt resistance is nearly already implemented in Shunt resistance light dependence #91 but I used RSH_E0 instead of RSH_STC and the light dependence is based on the SAM/CEC/DeSoto model

2. IMHO a more generic approach would be to implement an interface that allows the user to specify the model in pvconst. This was my recommendation in replace pvcell with abstract IV-curve API #39. That way the user could specify pvconst= PVconstants(model=pvlib.singlediode.bishop88) and it would use the existing methods from pvlib, rather than reimplenting them here. Your PR seems to be reimplenting the existing pvlib methods and making the existing 2-diode model more generic. While this may be a fine alternate approach, I think it will be more challenging to implement and maintain. That is the approach that I used in the MATLAB pvlife degradation model for SunPower: one model that could be used for 1-diode or 2-diode. There are pros and cons, and I'm okay with either approach ultimately. But in my opinion having an interface that allows the user to swap out the model for any external model will be easier and more flexible.

Related to #115, #39, #91

[cwhanse]

Something similar to your proposal for shunt resistance is nearly already implemented in #91 but I used RSH_E0 instead of RSH_STC and the light dependence is based on the SAM/CEC/DeSoto model

The current use of "0", "_0", "0_T0" etc. doesn't follow the patterns I'm used to seeing, which I'm trying to follow. For example, ISC0 is not Isc at STC conditions, it's Isc at reference irradiance only, requiring ISC0_T0 also. I'm hesitant to introduce another variant "E0". In the case of RSH, for the PVsyst one-diode model we need Rsh at STC and Rsh at 0 W/m2. I used RSH_STC and RSH_0 respectively, I suppose RSH_0 and RSH_dark (or something not RSH_0) would work. For the De Soto and CEC models, only Rsh at STC is needed.

IMHO a more generic approach would be to implement an interface that allows the user to specify the model in pvconst. This was my recommendation in #39.

I got the message from @chetan201 that minimal changes to the API are desired, since other applications are relying on PVMismatch. The pvlib functions have a very different API than this package. I would prefer not to duplicate functions already in pvlib, but that's not avoidable without a complete redo of pvcell.

That way the user could specify pvconst= PVconstants(model=pvlib.singlediode.bishop88) and it would use the existing methods from pvlib, rather than reimplenting them here.

I'm confused by this comment: bishop88 is a method to calculate an IV curve given values for the 5 parameters for a single diode equivalent circuit. It's not a single diode model, which in pvlib is implemented in the calcparams functions that intake irradiance and temperature and return the 5 parameters. The counterparts in PVMismatch are pvcell.calcCell (returns the IV curve from parameters) and the family of @property methods (e.g. Isat1, Isat2, etc. Are you suggesting that there's a convenient way to re-use e.g. pvlib.pvsystem.calcparams_desoto here?

But in my opinion having an interface that allows the user to swap out the model for any external model will be easier and more flexible.

I'm in complete agreement, but that's an overhaul of the pvcell API.

[mikofski]

@cwhanse thanks for your responses, all sounds good, let me know if/how I can support/help

[chetan201]

Hi @cwhanse, this is really cool! Just catching up on the changes and have started testing a bit.

@mikofski thanks a lot for your continued review support.

Thought I would share this absolute Pmp difference analysis here to hear your thoughts -

[mikofski]

I think this line should say elif diode_model == "pvsyst" right?

[mikofski]

I'm curious, does this line need if diode_model != '2-diode': return 0?

[mikofski]

❤️ thanks for using expm1

[mikofski]

Ah, re: my comment above I see now, it's fine to leave N2 as non-zero, because it won't be included in the desoto or pvsyst calcs anyway. Clever!

[mikofski]

I think here you could just remove N1 for 2-diode, and it could just be self.Eg / k_b * _inv_delta_T but I see, leaving it in gives it more flexibility, since N1 might not be one?

[mikofski]

Only one line L169 that looks like a typo to me, in the Rsh calculation, I think the second condition should say, if diode model is pvsyst, but it says desoto -- probably copy & paste typo. Otherwise this is amazing. Thank you x1e6 Cliff! I'm sorry you had to slog through my horrible code. I'm so embarrassed. 😕

[kandersolar]

@cwhanse Just curious, do you see this PR getting wrapped up in the next few weeks? I have something I'd like to use it for but can go another way if this is on the back burner. No pressure, I'm just trying to plan ahead.

[cwhanse]

@kanderso-nrel I could do that. A few corrections are needed, and tests. I stopped work when the tests became a problem. I cannot exactly reproduce the PVMismatch IV curves using pvlib functions, because PVMismatch parameterizes the curves using Isc rather than photocurrent. The difference in Isc from pvlib and PVMismatch is not small. Let me see if I can find the code to show the problem, I would appreciate someone else's eyes on this. It may be that I'm not using PVMismatch correctly, it also may be that the properties aren't being updated when I expected they are.