Handle poa_global and effective_irradiance for cell temperature models by cwhanse · Pull Request #1129 · pvlib/pvlib-python

cwhanse · 2021-01-11T19:09:36Z

Closes Irradiance for cell temperature models #1120 Irradiance for cell temperature models #1044
I am familiar with the contributing guidelines
Tests added
~~- [ ] Updates entries to docs/sphinx/source/api.rst for API changes.~~
Adds description and name entries in the appropriate "what's new" file in 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`).
New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.
Pull request is nearly complete and ready for detailed review.
Maintainer: Appropriate GitHub Labels and Milestone are assigned to the Pull Request and linked Issue.

For the irradiance input to cell temperature models, uses 'poa_global' if available in ModelChain.results.total_irrad, if not, uses ModelChain.results.effective_irradiance. Follow to #1076

temperature models

cwhanse · 2021-01-13T18:43:41Z

Ready for review. One question for discussion: when the code switches to use effective irradiance vs. 'poa_global' should the user be warned?

wholmgren

One question for discussion: when the code switches to use effective irradiance vs. 'poa_global' should the user be warned?

I lean no.

wholmgren · 2021-01-13T20:36:38Z

docs/sphinx/source/whatsnew/v0.9.0.rst

+  automatically switch to using 'effective_irradiance' (if available) for
+  cell temperature models, when 'poa_global' is not provided in input weather


Suggested change

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

wholmgren · 2021-01-13T20:43:03Z

pvlib/modelchain.py

    def fuentes_temp(self):
        return self._set_celltemp(self.system.fuentes_celltemp)

+    @staticmethod


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.

For less experienced python users (like me) it would be more familiar as a helper function.

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.

wholmgren · 2021-01-13T20:44:49Z

pvlib/modelchain.py

+
+        Returns
+        -------
+        None.


not accurate

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

wholmgren · 2021-01-13T20:45:34Z

pvlib/modelchain.py

+    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'


Suggested change

of preference 'poa_global' or 'effective_irradiance'

of preference 'poa_global' then 'effective_irradiance'

wholmgren · 2021-01-13T20:54:40Z

pvlib/tests/test_modelchain.py

    assert_series_equal(ac, expected)


+def test_run_model_from_effective_irradiance_no_poa_global(


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.

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.

…radiance

wholmgren · 2021-01-14T16:56:03Z

pvlib/modelchain.py

+
+    Returns
+    -------
+    Series of tuple of Series


Suggested change

Series of tuple of Series

Series or tuple of Series

wholmgren · 2021-01-14T16:58:53Z

pvlib/modelchain.py

        -----
+        Optional `data` columns ``'cell_temperature'``,
+        ``'module_temperature'`` and ``'poa_global'`` are used for determining
+        cell temperature.


not rendering as desired. might need a blank line here

https://pvlib-python--1129.org.readthedocs.build/en/1129/generated/pvlib.modelchain.ModelChain.run_model_from_effective_irradiance.html

wholmgren · 2021-01-14T16:59:00Z

pvlib/modelchain.py


        Notes
        -----
+        Optional `data` columns ``'cell_temperature'``,


Suggested change

Optional `data` columns ``'cell_temperature'``,

Optional ``data`` columns ``'cell_temperature'``,

cwhanse added 5 commits January 11, 2021 11:09

add switch between poa_global and effective_irradiance for cell

9317efb

temperature models

handle tuple and single DF

68b7cf7

handle tuples etc., improve some docstrings

8166cf6

more handle tuples

7e54534

add missing argument

40d8734

cwhanse added the enhancement label Jan 13, 2021

cwhanse added this to the 0.9.0 milestone Jan 13, 2021

whatsnew

c720a75

wholmgren reviewed Jan 13, 2021

View reviewed changes

cwhanse added 2 commits January 13, 2021 15:51

edits from review, add test for different poa_global and effective_ir…

2211fed

…radiance

move to helper function, add test

9174fe3

wholmgren approved these changes Jan 14, 2021

View reviewed changes

edits

8c3e643

wholmgren merged commit f94b5ad into pvlib:master Jan 14, 2021

cwhanse mentioned this pull request Jan 20, 2021

Irradiance for cell temperature models #1044

Closed

wholmgren mentioned this pull request Feb 5, 2021

ValueError: ModelChain.run_from_effective_irradiance([weather]) when only providing temp_air and wind_speed #1163

Closed

cwhanse mentioned this pull request Feb 6, 2021

fix run_model_from_effective_irradiance with iterable weather and excluding cell temperature #1165

Merged

8 tasks

		automatically switch to using 'effective_irradiance' (if available) for
		cell temperature models, when 'poa_global' is not provided in input weather

	of preference 'poa_global' or 'effective_irradiance'
	of preference 'poa_global' then 'effective_irradiance'

		assert_series_equal(ac, expected)


		def test_run_model_from_effective_irradiance_no_poa_global(

	Optional `data` columns ``'cell_temperature'``,
	Optional ``data`` columns ``'cell_temperature'``,

Conversation

cwhanse commented Jan 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cwhanse commented Jan 13, 2021

Uh oh!

wholmgren left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cwhanse commented Jan 11, 2021 •

edited

Loading