Clarify which SAPM parameters are required by module in pvsystem.sapm
#2435
+15
−13
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
RDaxini
changed the title
module argument of pvsystem.sapmmodule in pvsystem.sapm
cwhanse
reviewed
Apr 10, 2025
echedey-ls
reviewed
Apr 11, 2025
cwhanse
approved these changes
Apr 16, 2025
Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>
echedey-ls
reviewed
Apr 18, 2025
There was a problem hiding this comment.
The following module keys are being used in pvlib.pvsystem.sapm, module parameter (do not match with current text):
Ordering by when they appear in code
Bvmpo
Mbvmp
Bvoco
Mbvoc
N
Cells_in_Series
Isco
Aisc
Impo
C0
C1
Aimp
Voco
Vmpo
C2
C3
IXO
C4
C5
Aisc
IXXO
C6
C7
Aimp
Ordering same as table
C0-C7
Isco
Impo
Voco
Vmpo
Aisc
Aimp
Bvoco
Mbvoc
Bvmpo
Mbvmp
N
Cells_in_Series
IXO
IXXO
Two points:
- It's usually better to speak/reflect statements in positive, so no ambiguities are left and in this case you also save time to the end user by not getting the full list, then excluding the list of unused parameters.
- While this PR does address the linked issue, I find the optimal solution would also be to document in the
effective_irradianceparameter, that the effective irradiance is calculated by using the other sapm function(s) (which right now are not listed in the See Also section, adding them too would be a nice improvement)
EDIT: I see this review does not partially apply due to PR #2433, I did not see it earlier.
Comment on lines
+2235
to
+2238
| Both databases can be accessed using :py:func:`retrieve_sam`. The full list | ||
| of SAPM parameters is presented in the table below. All but ``A0`` to | ||
| ``A4``, ``B0`` to ``B5``, ``C4`` to ``C7``, ``IXO``, and ``IXXO`` are | ||
| required in the ``module`` parameter. |
There was a problem hiding this comment.
Suggested change
| Both databases can be accessed using :py:func:`retrieve_sam`. The full list | |
| of SAPM parameters is presented in the table below. All but ``A0`` to | |
| ``A4``, ``B0`` to ``B5``, ``C4`` to ``C7``, ``IXO``, and ``IXXO`` are | |
| required in the ``module`` parameter. | |
| Both databases can be accessed using :py:func:`retrieve_sam`. The full list | |
| of SAPM parameters is presented in the table below. | |
| The following subset of the SAPM module parameters is required to run this | |
| model: | |
| * ``C0-C7`` | |
| * ``Isco`` | |
| * ``Impo`` | |
| * ``Voco`` | |
| * ``Vmpo`` | |
| * ``Aisc`` | |
| * ``Aimp`` | |
| * ``Bvoco`` | |
| * ``Mbvoc`` | |
| * ``Bvmpo`` | |
| * ``Mbvmp`` | |
| * ``N`` | |
| * ``Cells_in_Series`` | |
| * ``IXO`` | |
| * ``IXXO`` |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
pvsystem.sapmdocumentation: clarification of coefficients used #2392Tests addedUpdates entries indocs/sphinx/source/referencefor API changes.docs/sphinx/source/whatsnewfor all changes. Includes link to the GitHub Issue with:issue:`num`or this Pull Request with:pull:`num`. Includes contributor name and/or GitHub username (link with:ghuser:`user`).remote-data) and Milestone are assigned to the Pull Request and linked Issue.