Handle poa_global and effective_irradiance for cell temperature models #1129
Conversation
|
Ready for review. One question for discussion: when the code switches to use effective irradiance vs. 'poa_global' should the user be warned? |
| automatically switch to using 'effective_irradiance' (if available) for | ||
| cell temperature models, when 'poa_global' is not provided in input weather |
There was a problem hiding this comment.
| automatically switch to using 'effective_irradiance' (if available) for | |
| cell temperature models, when 'poa_global' is not provided in input weather | |
| automatically switch to using ``'effective_irradiance'`` (if available) for | |
| cell temperature models, when ``'poa_global'`` is not provided in input weather |
pvlib/modelchain.py
Outdated
| @@ -1041,6 +1043,28 @@ def faiman_temp(self): | |||
| def fuentes_temp(self): | |||
| return self._set_celltemp(self.system.fuentes_celltemp) | |||
|
|
|||
| @staticmethod | |||
There was a problem hiding this comment.
We have a few helper functions at the bottom of the module. I wonder if this should live there rather than be a staticmethod on ModelChain. I don't have a strong preference at this time.
There was a problem hiding this comment.
For less experienced python users (like me) it would be more familiar as a helper function.
There was a problem hiding this comment.
Well, I had to lookup staticmethod in the python documentation to confirm its behavior before reviewing the function, count me in the less experienced group too.
pvlib/modelchain.py
Outdated
|
|
||
| Returns | ||
| ------- | ||
| None. |
There was a problem hiding this comment.
One question for discussion: when the code switches to use effective irradiance vs. 'poa_global' should the user be warned?
I lean no.
I also, but the switch should be made clear in the ModelChain documentation, and in the docstring for ModelChain.run_model_from_effective_irradiance
pvlib/modelchain.py
Outdated
| def _irrad_for_celltemp(total_irrad, effective_irradiance): | ||
| """ | ||
| Determine irradiance to use for cell temperature models, in order | ||
| of preference 'poa_global' or 'effective_irradiance' |
There was a problem hiding this comment.
| of preference 'poa_global' or 'effective_irradiance' | |
| of preference 'poa_global' then 'effective_irradiance' |
| @@ -918,6 +918,18 @@ def test_run_model_from_effective_irradiance(sapm_dc_snl_ac_system, location, | |||
| assert_series_equal(ac, expected) | |||
|
|
|||
|
|
|||
| def test_run_model_from_effective_irradiance_no_poa_global( | |||
There was a problem hiding this comment.
Do we also need a test for when both effective irradiance and poa global are provided? I think not but thought it was worth mentioning.
There was a problem hiding this comment.
I think it's worth adding this test. All of the tests for run_model_from_effective_irradiance copy the values for 'poa_global' to 'effective_irradiance' (for convenience) so can't distinguish if the wrong values are being used to calculate power.
pvlib/modelchain.py
Outdated
|
|
||
| Returns | ||
| ------- | ||
| Series of tuple of Series |
There was a problem hiding this comment.
| Series of tuple of Series | |
| Series or tuple of Series |
| @@ -1740,6 +1751,19 @@ def run_model_from_effective_irradiance(self, data=None): | |||
|
|
|||
| Notes | |||
| ----- | |||
| Optional `data` columns ``'cell_temperature'``, | |||
| ``'module_temperature'`` and ``'poa_global'`` are used for determining | |||
| cell temperature. | |||
There was a problem hiding this comment.
not rendering as desired. might need a blank line here
pvlib/modelchain.py
Outdated
| @@ -1740,6 +1751,19 @@ def run_model_from_effective_irradiance(self, data=None): | |||
|
|
|||
| Notes | |||
| ----- | |||
| Optional `data` columns ``'cell_temperature'``, | |||
There was a problem hiding this comment.
| Optional `data` columns ``'cell_temperature'``, | |
| Optional ``data`` columns ``'cell_temperature'``, |
- [ ] Updates entries todocs/sphinx/source/api.rstfor 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`).For the irradiance input to cell temperature models, uses 'poa_global' if available in
ModelChain.results.total_irrad, if not, usesModelChain.results.effective_irradiance. Follow to #1076