Skip to content

Error in Ixx equation in pvsystem.sapm #2016

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
cwhanse opened this issue Apr 22, 2024 · 1 comment · Fixed by #2019
Closed

Error in Ixx equation in pvsystem.sapm #2016

cwhanse opened this issue Apr 22, 2024 · 1 comment · Fixed by #2019
Labels
Milestone

Comments

@cwhanse
Copy link
Member

cwhanse commented Apr 22, 2024

The SAPM function uses Aisc (temperature coefficient for Isc) in the equation for Ixx, where it should be Aimp (temperature coefficient for Imp). This line.

Equation 10 of the 2004 Sandia report shows Aimp is intended.

Discussion among the Sandia staff concluded that there is a typo in PVLib for Matlab, in this line.

This error was propagated when the Matlab code was ported to python by R. Andrews (see pvl_sapm.py file in the prearizona tag for pvlib).

The comment originated with W. Holmgren, who may have been looking at the Matlab code as the authoritative source.

Expected behavior
Correct this line to use Aimp.

Versions:

  • pvlib.__version__: all versions

Additional context
The error only affects Ixx from pvsystem.sapm and nothing else.

PVLib for Matlab issue sandialabs/MATLAB_PV_LIB#37

@cwhanse cwhanse changed the title Error Ixx equation in pvsystem.sapm Error in Ixx equation in pvsystem.sapm Apr 22, 2024
@kandersolar kandersolar added this to the v0.10.5 milestone Apr 29, 2024
@kandersolar
Copy link
Member

Every reference for these equations that I was able to locate with reasonable effort supports the conclusion that the equation should use Aimp.

I also agree that the error is limited to the line of code linked above.

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