Adds faiman_rad and ross models to get_cell_temperature(). by ramaroesilva · Pull Request #2631 · pvlib/pvlib-python

ramaroesilva · 2025-12-18T23:21:52Z

Closes pvlib.pvsystem.Array.get_cell_temperature does not accept ross and faiman_rad models #2625
I am familiar with the contributing guidelines
Tests added
[] Updates entries in docs/sphinx/source/reference 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 (including remote-data) and Milestone are assigned to the Pull Request and linked Issue.

Differently from what was originally discussed with @cwhanse and @echedey-ls, I decided not to include a verification on whether both k and noct are provided within get_cell_temperature. imo, this type of verification should be (and is) done within the temperature.ross function itself and we see that the opposite situation - expected parameters not being provided - is never verified within get_cell_temperature as I imagine it is done within each temperature model function.

@cwhanse about wind and ross model, if that's okay I still included a small note in the wind_speed description to alert ross users that providing a None is enough.

ramaroesilva · 2025-12-18T23:27:37Z

pvlib/pvsystem.py


-        temperature_cell = func(poa_global, temp_air, wind_speed,
-                                *required, **optional)
+        if model == 'ross':


Although wind speed was kept as an input requested by get_cell_temperature, here such a differentiation is needed since ross is apparently the only model not using wind.

@cwhanse @echedey-ls this if ross can be avoided if for the wind-using functions (all except ross), wind_speed is defined as input as below (example for one function):

# func = temperature.sapm_cell # to deprecate func = functools.partial(temperature.sapm_cell, wind_speed=wind_speed)

and the if ross with two possibilities is replaced by the single line below

temperature_cell = func(poa_global, temp_air, *required, **optional)

ramaroesilva · 2025-12-21T13:47:42Z

Have included tests for the added temperature models.

@adriesse, for the faiman_rad case could you suggest one set of example ir_down/u0/y1 values for testing? I'm asking this because of this note on the faiman_rad docstring:

When only irradiance, air temperature and wind speed inputs are provided (ir_down is None) this function calculates the same device temperature as the original faiman model. When down-welling long-wave radiation data are provided as well (ir_down is not None) the default u0 and u1 values from the original model should not be used because a portion of the radiative losses would be double-counted.

Afterwards, I can get the expected output by running pvlib.temperature.faiman_rad for poa_global=1000, temp_air=25, wind_speed=1, and your recommended parameter values.

kandersolar · 2025-12-22T14:05:24Z

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

 * Add :py:func:`~pvlib.spectrum.spectral_factor_polo`, a function for estimating
  spectral mismatch factors for vertical PV façades. (:issue:`2406`, :pull:`2491`)
+* Includes `ross` and `faiman_rad` in the allowed models within 
+  `pvlib.pvsystem.PVSystem.get_cell_temperature` (:issue:`2625`, :pull:`2631`)


Suggested change

`pvlib.pvsystem.PVSystem.get_cell_temperature` (:issue:`2625`, :pull:`2631`)

:py:meth:`pvlib.pvsystem.PVSystem.get_cell_temperature` (:issue:`2625`, :pull:`2631`)

kandersolar · 2025-12-22T14:05:42Z

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

  MERRA-2 reanalysis data. (:pull:`2572`)
 * Add :py:func:`~pvlib.spectrum.spectral_factor_polo`, a function for estimating
  spectral mismatch factors for vertical PV façades. (:issue:`2406`, :pull:`2491`)
+* Includes `ross` and `faiman_rad` in the allowed models within 


Suggested change

* Includes `ross` and `faiman_rad` in the allowed models within

* Include `ross` and `faiman_rad` in the allowed models within

kandersolar · 2025-12-22T14:10:51Z

pvlib/pvsystem.py


        wind_speed : numeric
-            Wind speed [m/s]
+            Wind speed [m/s], although can be ``None`` for ``'ross'`` model


Should we add this to the corresponding PVSystem description too?

do you mean within the function below?

@_unwrap_single_value
def get_cell_temperature(...)

@kandersolar was this point blocking the merge of this PR for v0.14.0? Should have checked #2656 to provide more timely feedback.

Not really. This PR is additive (i.e. not a breaking change), so I just didn't prioritize coming back to review it again in time for v0.14.0. We can get this PR into v0.14.1 no problem.

do you mean within the function below?

I meant adding this same note about "can be zero for Ross" to this description as well: https://github.com/ramaroesilva/pvlib-python/blob/pvsystem-temp-models/pvlib/pvsystem.py#L429-L430

kandersolar · 2025-12-22T14:15:43Z

pvlib/pvsystem.py

+        elif model == 'faiman_rad':
+            func = temperature.faiman_rad
+            required = ()
+            optional = _build_kwargs(['ir_down', 'u0', 'u1',


ir_down being a value in temperature_model_parameters seems wrong to me. Shouldn't it be an optional time series input like effective_irradiance is?

You're totally right. Already handled this in my latest commits. Thanks!

adriesse · 2025-12-22T16:44:54Z

Have included tests for the added temperature models.

@adriesse, for the faiman_rad case could you suggest one set of example ir_down/u0/y1 values for testing? I'm asking this because of this note on the faiman_rad docstring:

When only irradiance, air temperature and wind speed inputs are provided (ir_down is None) this function calculates the same device temperature as the original faiman model. When down-welling long-wave radiation data are provided as well (ir_down is not None) the default u0 and u1 values from the original model should not be used because a portion of the radiative losses would be double-counted.

Afterwards, I can get the expected output by running pvlib.temperature.faiman_rad for poa_global=1000, temp_air=25, wind_speed=1, and your recommended parameter values.

For the test it doesn't really matter what the values are, and what you've done seems fine. Not sure if you want to keep ir_down a scalar as discussed above. In practice the reduction in U values will depend on the ir_down conditions at the location.

cwhanse

@ramaroesilva this fell off my radar, sorry. I don't have an opinion about the ross wind_speed accommodation. The if structure is OK with me, it's easy for a functools novice to understand :)

ramaroesilva added 2 commits December 18, 2025 23:11

Adds faiman_rad and ross models to get_cell_temperature().

4143e2a

Addresses flake8 formatting issues.

70bb948

ramaroesilva commented Dec 18, 2025

View reviewed changes

Updates contribution md file.

acc43b4

ramaroesilva added 8 commits December 21, 2025 13:52

Adds test for ross option, placeholder for faiman_rad.

69c7f43

Corrects flake8 issues.

5998725

Adds missing wind_speed=None when testing get_cell_temperature for ross.

61e8163

Corrects wrong function naming.

22f94a7

Adds missing input to functions within tests.

aaa52e3

Corrects test + flake8 issue.

c6d8ab9

Minor update in documentation + flake8 issue.

78eb4c3

Another flake8 issue handled.

402b53c

cwhanse approved these changes Dec 22, 2025

View reviewed changes

ramaroesilva added 2 commits December 22, 2025 08:09

Improve faiman_rad test and completes get_cell_temperature doc.

9461e01

Addresses flake8.

9035066

kandersolar reviewed Dec 22, 2025

View reviewed changes

ramaroesilva added 3 commits December 22, 2025 16:04

Update whatsnew doc.

6f5bc0d

Defines ir_down as weather variable Within faiman_rad.

a536cb2

Handles flake8 issues.

bc62578

ramaroesilva added 2 commits December 22, 2025 22:55

Corrects bug in test.

7612e56

Handles flake8.

534e2f6

cwhanse approved these changes Jan 17, 2026

View reviewed changes

kandersolar added this to the v0.14.1 milestone Jan 17, 2026

kandersolar modified the milestones: v0.14.1, v0.15.1 Jan 26, 2026

	`pvlib.pvsystem.PVSystem.get_cell_temperature` (:issue:`2625`, :pull:`2631`)
	:py:meth:`pvlib.pvsystem.PVSystem.get_cell_temperature` (:issue:`2625`, :pull:`2631`)

	* Includes `ross` and `faiman_rad` in the allowed models within
	* Include `ross` and `faiman_rad` in the allowed models within

Conversation

ramaroesilva commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ramaroesilva commented Dec 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

ramaroesilva Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adriesse commented Dec 22, 2025

Uh oh!

cwhanse left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ramaroesilva commented Dec 18, 2025 •

edited

Loading

ramaroesilva commented Dec 21, 2025 •

edited

Loading

ramaroesilva Dec 22, 2025 •

edited

Loading