Update Hercules solar model to accept pysam_options dictionary#250
Conversation
| print( | ||
| "PySAM model options provided in input are being used to define the PVWatts system." | ||
| ) | ||
| if "losses" in h_dict[self.component_name] or "tilt" in h_dict[self.component_name]: | ||
| print( | ||
| "Warning: 'losses' and 'tilt' parameters in the input will be ignored " | ||
| "since PySAM model options are provided." | ||
| ) |
There was a problem hiding this comment.
For consistency should use self.logger.info in place of print
There was a problem hiding this comment.
And on the warning statement self.logger.warning()
There was a problem hiding this comment.
Done! One is updated for a logger.info and the other was changed to a ValueError
| sys_design = { | ||
| "ModelParams": {"SystemDesign": h_dict[self.component_name]["pysam_options"]}, | ||
| } | ||
|
|
There was a problem hiding this comment.
Do we need a check that sys_design contains some minimum number of defined parameters? We could also supply defaults through an |
There was a problem hiding this comment.
Done! Let me know what you think!
There was a problem hiding this comment.
yes, that looks really good! I had a few more very small comments but all in all great!
| log_channels: | ||
| - power | ||
|
|
||
| # solar_farm: # The name of component object 1 |
There was a problem hiding this comment.
Let's remove commented code before final merge
| } | ||
|
|
||
| # Ensure system capacity is set from upper level of input | ||
| # TODO: Should losses and tilt also be forced to be set from |
There was a problem hiding this comment.
What if we say, everything that is in effect implemented through sys_design must come in through pysam_options to make things less confusing
There was a problem hiding this comment.
Hmm, I was trying to differentiate between the pysam_options definition and the Hercules defaults. We could have defaults for some parameters in pysam_options and not others, but that might obscure what you can actually set. Alternatively, we could require the users to set all of the necessary parameters every time. Thoughts?
There was a problem hiding this comment.
My idea was if a parameter is only used by Hercules as a pass-through to pvwatts, it goes into the pysam_options sub_dict, otherwise if it's used at least one other place within Hercules can be higher level and copied in
|
Thank you @genevievestarke so much for jumping on this!! Had a few comments for your consideration and happy to help with any pieces like logging etc., |
2 participants
This PR allows the pysam PVWatts model in Hercules to accept a dictionary that can define the parameters in the
SystemDesignportion of the model. A good reference for what you can set using this dictionary can be found here: https://h2integrate.readthedocs.io/en/stable/technology_models/pvwattsv8_solar_pv.htmlThis is fully back compatible with current configurations, but it now allows the solar farm parameters to be set with more precision if needed.
To Do:
Tests to add: