-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Implement hybrid SAPM+Campanelli et al. model #475
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
Comments
@thunderfish24 if I am following your ideas, you are proposing to replace the current If we could streamline the I wonder how we'd deal with the necessary model parameters which are different among, and specific to, each single diode model (e.g., Desoto has only |
I like the idea of a wrapper function as we implement more |
@cwhanse @wholmgren I am not necessarily proposing to replace all the As Cliff also points out, I think the assembly of the corresponding |
So to be clear, the word "hybrid" in |
I'll admit I'm intrigued with the idea that we can wrap the calcparams and specify the model with a keyword. I think the challenge will be to manage the model parameters. There's no getting around the fact that each model has some unique values, and that the parameter sets we have to offer are only for one of the diode models (the CEC model). I think the flexibility to mix and match auxiliary equations has value but not to most users. There is value is our current API for most users. Using separate functions for each model forces us to be explicit and transparent about the equations are used in each model - and these are primary objectives of pvlib. So despite the intriguing nature of the wrapper, I'm hesitant to say let's replace the 'one calcparams per model' approach. For the 'hybrid' model you propose, I'm ok with that as long as there is a published reference documenting the model being submitted. |
#478 has working code now. Guinea pigs welcome! Based on the development in |
I'll echo Cliff's comments:
The **kwargs approach outlined here is a totally valid and very powerful for some use cases. But I am having a hard time reconciling it with the approach taken by pvlib so far.
I don't really understand the connection to dask/dask.delayed. Those are flexible frameworks and, in my experience, work fine with the existing pvlib api. On a related note, my experience with using dask/dask.delayed is that it's even more important to use very explicit code in those frameworks because of the additional debugging challenges they introduce. |
@wholmgren I am now familiarizing myself with the ModelChain as I evaluate architecture options for adding a new flavor of diode model to pvlib. Is the statement about ModelChain here still valid: "CEC module specifications and the single diode model are not yet supported."? The Dask |
@wholmgren I think it might be wise to open a separate issue to determine how we are going to implement the various single diode models such as pvsyst, first_solar, and campanelli so that they all integrate into |
We need a |
If I understand correctly, I think the standard pvlib approach would work and I don't think we'd need custom
We would want to decouple |
By custom
I agree that the |
I would like to implement a hybrid model that combines the single-diode model (SDM) developments of Campanelli et al. (see this) with the SAPM to compute the effective irradiance ratio F and effective temperature ratio H using the SAPM when necessary, i.e., when not using a matched reference device or cell-embedded junction temperature measurement.
I figure one should embrace Python's duck typing (Quack!), so below is my proposed SDM 5 coefficient calculation architecture that is a partial generalization of the existing function
calcparams_desoto
. Basically, this is a way of implementing a (somewhat inelegant) computational graph for all the coefficients (e.g., IL, I0, Rs, Rsh, nNsVth). I realize many folks aren't familiar with my model, but if you work through this, you'll see that something like this pattern may make it easier for users to implement different "flavors" of auxiliary equations for the SDM when they are trying to compare models, find a better model, and the like. (I may need to recast my model in terms ofeffective_irradiance
andcell_temp
to make it fit better with the current direction of the pvlib API, and this might make possible more reuse of auxiliary equations between different SDMs.)There is a lot of metadata packed into
kwargs
, and this provided metadata would change with the particular auxiliary equations selected. (I apologize that this will be a bit hard to understand from just the code snippet below, but I am still working on a runnable example.) What's nice about this pattern, is that one can specify a scalar, an array, or a function-to-be-computed for F, H, and IL, I0, Rs, Rsh, nNsVth, and once a parameter is computed, it can then "flow through" to the remaining computations. (However, unlike Dask, the user has to specify the computation in the proper (serial) order to avoid missing computations or re-computations of the computational graph's nodes.)The text was updated successfully, but these errors were encountered: