-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add loss input to example in modelchain.with_pvwatts docs #1863
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
Add loss input to example in modelchain.with_pvwatts docs #1863
Conversation
>>> pvwatts_losses = {'soiling': 2, 'shading': 3, 'snow': 0, 'mismatch': 2, | ||
>>> 'wiring': 2, 'connections': 0.5, 'lid': 1.5, | ||
>>> 'nameplate_rating': 1, 'age': 0, 'availability': 30} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should have two examples here, one that keeps things as simple as possible, and one that gets more into the weeds (like specifying losses)? with_pvwatts
is an easy point of entry for ModelChain, and PVWatts isn't really the model to use if you care a lot about the details, so I'm hesitant to make the only usage example more complicated.
IMHO there is also an argument to be made that PVWatts losses aren't really legitimate on their own and are better regarded as a single combined fudge factor, but maybe that's just my editorializing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think users should ever run a model without any considerations for what losses are specified or at least be aware that losses are accounted for by the model? Acknowledging and accepting the default values is perfectly resonable, but awareness of what they are seems pertinent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have created two examples, one simple and one that builds upon the simple one where the losses are modified.
Let me know if it warrants a whatsnew entry (I don't think it does).
pvlib/modelchain.py
Outdated
The PVWatts model defaults to 14 % total system losses. The loss | ||
assumptions can be modified as shown in the example below. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PVWatts model defaults to 14 % total system losses. The loss | |
assumptions can be modified as shown in the example below. | |
The PVWatts model defaults to 14 % total system losses. The loss | |
assumptions represent annual averages and can be modified as | |
shown in the example below. | |
The model is not recommended for high-accuracy applications. |
Thoughts on this edit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two thoughts, neither of which I feel too strongly about:
- I think "annual averages" is true but not as specific as it could be. How about "...represent constant DC-side loss fractions and can be modified..."? As an aside, constant losses means all simulation timesteps are affected equally (on a relative basis), so the "snow loss" applies to July just as much as it does to December...
- Re "not recommended": I certainly don't disagree with the sentiment, but this struck me as too subjective a statement to be appropriate for pvlib's documentation. If we want to make novice users aware that PVWatts v5 isn't a very detailed model, I think it would be better to describe what the model does (or doesn't) do, and let the user judge suitability for themselves.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bikeshedding: "The PVWatts losses are fractions of DC power and can be modified..."
[ ] Closes #xxxx[ ] Tests added[ ] Updates entries indocs/sphinx/source/reference
for API changes.docs/sphinx/source/whatsnew
for 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.I had some difficulties figuring out what losses were used in the
modelchain.with_pvwatts
, I am making this PR to make it clearer and demonstrate how to change the loss factors.My motivation for this PR is that I think it's important to encourage users to consider what loss assumptions are made and make it easier to modify them.
Also, it's typically new users who would use the
modelchain.with_pvwatts
model, which is why I have added the necessary imports such that the example can be run exactly as copied without having users to figure out where the different modules are imported from (something that would be difficult for a new user).