Enable custom detailed PV layout models #112
Conversation
|
|
@Matthew-Boyd Apologies for the delay. Looking at this now. I don't think detailed_pv_layout.py and detailed_pv_config.py were intended to be merged into main HOPP since they're not complete. Do you think it makes sense to not include them in the main HOPP or would something be lacking? Examples could be a good idea, but I think it would need to be more functional / complete. |
dguittet
left a comment
dguittet
left a comment
There was a problem hiding this comment.
I'm really liking the capability to pass in instantiated component classes like layout.
hybrid/layout/pv_design_utils.py
Outdated
|
|
||
|
|
||
| def get_module_attribs(pvsam_model: pv.Pvsamv1) -> dict: | ||
| def get_module_attribs(pvsam_model: pv_detailed.Pvsamv1) -> dict: |
There was a problem hiding this comment.
Would be really nice to have a docstring here linking to the attributes in the PySAM documentation
hybrid/layout/pv_design_utils.py
Outdated
| 'I_mp_ref': I_mp, | ||
| 'I_sc_ref': I_sc, | ||
| 'P_mp_ref': P_mp, | ||
| 'P_mp_ref': P_mp * 1e-3, |
There was a problem hiding this comment.
could you add units to these key-value pairs too as you did below?
hybrid/layout/pv_layout.py
Outdated
| """ | ||
| if type(self.parameters) == PVGridParameters: | ||
| self.reset_solargrid(size_kw, self.parameters) | ||
| if type(self.parameters) == PVGridParameters or "DetailedPVParameters" in str(type(self.parameters)): |
There was a problem hiding this comment.
If someone named their parameters differently, then this code wouldn't be flexible enough to handle it. I think we should throw an error to stave off confusion
There was a problem hiding this comment.
I'm not sure now why I even needed to accommodate the DetailedPVParameters type, so I just removed it. I think the tests will pass, we'll see.
@dguittet No worries, I got to catch up on other work. I'm content putting them into examples as-is now, and let Engie use this right away. Maybe they would offer comments or a PR during their testing and implementation? I'd rather not let the changes in this PR languish, however, so if you thought it best to develop those two files more, maybe we can still merge the rest? (I would have to remove the affected tests, though.) |
|
@Matthew-Boyd Thanks I've moved the detailed layout files into the examples folder |
|
@dguittet Good to merge? |
Enables custom detailed PV layout models for both the detailed (pvsamv1) and simple (pvwattsv8) technology models.