Skip to content

Conversation

@kandersolar
Copy link
Member

@kandersolar kandersolar commented Nov 3, 2025

  • [ ] Closes #xxxx
  • 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.

Continuing in a broader thrust of developing our user guide section. Previously:

See also:

Docs build: https://pvlib-python--2591.org.readthedocs.build/en/2591/user_guide/modeling_topics/temperature.html

@kandersolar kandersolar added this to the v0.13.2 milestone Nov 3, 2025
@adriesse
Copy link
Member

adriesse commented Nov 4, 2025

One thing that could be added is the parameter conversion from one model to another. Please leave this open for a few weeks as I'd like to comment more.

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>
@AdamRJensen
Copy link
Member

Could consider listing prilliman in the table with only a check mark for the dynamic column and a dash or a blank cell for the remaining columns. I'm open to other suggestions, but I think it's a shame that it's hidden as a footnote at the end of the guide.

@kandersolar kandersolar mentioned this pull request Dec 1, 2025
5 tasks
@AdamRJensen
Copy link
Member

This PR has been open for a few weeks now. I suggest we merge it on Monday the 7th if there are no objections.

can be converted to another model (e.g. ``u_c``, ``u_v`` for :py:func:`~pvlib.temperature.pvsyst_cell`)
using :py:class:`~pvlib.temperature.GenericLinearModel`.

Currently, pvlib provides no functionality for fitting parameter values
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could consider including references to:

  • The sandia data set for temperature model fitting, and it's accompanying notebook
  • My paper on model fitting from last years PVSEC.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure pointing to external functionality makes sense for these pages. Let's save that for a potential follow-up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What kind of "sense" are we looking for? Pointing to solutions takes the edge off the foregoing limitation a bit. I think the new spectrum section has similar pointers. But I leave the decision to you.

Copy link
Contributor

@IoannisSifnaios IoannisSifnaios left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

kandersolar and others added 3 commits December 3, 2025 09:07
Co-authored-by: Anton Driesse <anton.driesse@pvperformancelabs.com>
Co-Authored-By: Ioannis Sifnaios <88548539+IoannisSifnaios@users.noreply.github.com>
can be converted to another model (e.g. ``u_c``, ``u_v`` for :py:func:`~pvlib.temperature.pvsyst_cell`)
using :py:class:`~pvlib.temperature.GenericLinearModel`.

Currently, pvlib provides no functionality for fitting parameter values
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What kind of "sense" are we looking for? Pointing to solutions takes the edge off the foregoing limitation a bit. I think the new spectrum section has similar pointers. But I leave the decision to you.

+----------------------------------------------+--------+------------+----------------+---------------------+------------+-----------------------+
| :py:func:`~pvlib.temperature.fuentes` | either ||||| |
+----------------------------------------------+--------+------------+----------------+---------------------+------------+-----------------------+
| :py:func:`~pvlib.temperature.generic_linear` | cell | |||| |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the doc string says cell temperature, but the model is really agnostic as in principle all the others are too. I recommend putting "either" here possibly flag the docstring to be changed. In the class description module temperature is used.

Copy link
Contributor

@echedey-ls echedey-ls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very informative. LGTM. I don't feel like I would have any questions after reading it, within scope of these docs.

Comment on lines +22 to +31
Temperature models estimate these quantities using inputs like incident
irradiance, ambient temperature, and wind speed. Each model also takes
a set of parameter values that represent how a PV module responds to
those inputs. Parameter values generally depend on both the PV
module technologies, the mounting configuration of the module,
and on any weather parameters that are not included in the model.
Note that, despite models conventionally being associated with either
cell or module temperature, it is actually the parameter values that determine
which of the two temperatures are predicted, as they will produce the same
type of temperature from which they were originally derived.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest splitting this in two paragraphs, e.g. at line 25.

following table:

+----------------------------------------------+--------+------------+---------------------------------------------------------------------------+
| Model | Type | Transient? | Inputs |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| Model | Type | Transient? | Inputs |
| Model | Type | Transient? | Weather inputs |

There are many other unlisted inputs that could become a dealbreaker when selecting a model. For the sake of precision.

I also wonder whether those two fully-tick-marked columns are really adding some value to the reader, but I don't see much room for edits in there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants