Skip to content

consistency with matlabs sapm api #198

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

Closed
wholmgren opened this issue Jun 23, 2016 · 5 comments · Fixed by #218
Closed

consistency with matlabs sapm api #198

wholmgren opened this issue Jun 23, 2016 · 5 comments · Fixed by #218
Labels
Milestone

Comments

@wholmgren
Copy link
Member

The Matlab project's sapm function signature is:

pvl_sapm(Module, Ee, celltemp)

while ours is

sapm(module, poa_direct, poa_diffuse, temp_cell, airmass_absolute, aoi)

The Matlab project has used that signature since at least version 1.1 (the oldest that I have). The Python signature started as

pvl_sapm(Module,Eb,Ediff,Tcell,AM,AOI)

in commit e74644b and I changed it to the current signature in 5f9bd7c. If I'm reading the original commit correctly (specifically line 101), I think the units of the irradiance should be W/m**2 despite the docstring's claim of suns.

So that's the current status and history, and I will leave it to you all to decide how you want to go forward.

@wholmgren wholmgren added the api label Jun 23, 2016
@wholmgren wholmgren added this to the 0.4.0 milestone Jun 23, 2016
@cwhanse
Copy link
Member

cwhanse commented Jun 24, 2016

We chose to use Ee (effective irradiance) as input to SAPM in Matlab so that we had flexibility to replace the reflection and spectral mismatch functions. Ee can be computed in a variety of ways from POA_direct and POA_diffuse. I would encourage doing similarly in python.

@wholmgren
Copy link
Member Author

It appears to me that the Matlab library relies on the user to implement the calculations below instead of providing functions such as pvl_sapm_spectral_correction, pvl_sapm_aoi, and pvl_sapm_effective_irradiance. Correct?

F1 = max(0,polyval(ModuleParameters.a,AMa)); %Spectral loss function
F2 = max(0,polyval(ModuleParameters.b,AOI)); % Angle of incidence loss function
Ee = F1.*((Eb.*F2+ModuleParameters.fd.*Ediff)/E0)*SF; %Effective irradiance

Perhaps the Python library should adopt the pvl_sapm api and add some simple functions to standardize the SAPM algorithms. I'm trying to avoid doing any real calculations in ModelChain, so I'd need new functions/methods to call.

@cwhanse
Copy link
Member

cwhanse commented Jun 24, 2016

Yes, that’s correct. We felt that approach would offer more flexibility to a modeler who preferred the Martin and Ruiz AOI model, for example, over the Sandia polynomial. We take the same approach with the single diode models, i.e., the input to these functions is Ee.

@wholmgren
Copy link
Member Author

Ok, I'm sold. While I liked the ease of use with sapm taking care of that for me, I disliked the inconsistency with the single diode models. Now that we have PVSystem and ModelChain for the ease of use issues, we should keep the functions simple and consistent.

For what it's worth, I pushed a couple of new-but-old https://github.com/pvlib/pvlib-python/tags to help us keep track of why things are the way that they are and thus better understand the implications of changing things.

@wholmgren
Copy link
Member Author

Another point on consistency:

pvl_sapm takes Ee in units of suns.
pvl_calcparams_desoto takes S in units of W/m^2.
pvl_calcparams_PVsyst takes Ee in units of W/m^2.
pvl_calcparams_CEC takes S in units of W/m^2.

I'd prefer if the Python functions all take effective_irradiance in units of W/m^2. The discussion in #195 makes me think e_poa_transmitted would work too.

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

Successfully merging a pull request may close this issue.

2 participants