Clarify rh_from_tdew variable naming, comments and references#2782
Open
gaoflow wants to merge 2 commits into
Open
Clarify rh_from_tdew variable naming, comments and references#2782gaoflow wants to merge 2 commits into
gaoflow wants to merge 2 commits into
Conversation
In rh_from_tdew the intermediate named `e` was computed from the air temperature (so it was actually the saturation vapor pressure) and `es` from the dew point (the actual vapor pressure) -- swapped relative to the usual convention, which led pvlib#2734 to read it as an inverted formula. The returned values are correct and unchanged (100 * es/e with the swapped names equals 100 * e/es with conventional ones); this computes `e` from the dew point and `es` from the air temperature so the source matches the convention, and fixes the matching derivation comment in tdew_from_rh. Also adds a Notes section and the Alduchov & Eskridge (1996) reference for the Magnus form, plus a physical-bounds regression test (saturation when dew point == air temperature, RH below 100% and monotonic in the dew point otherwise) so the direction cannot silently invert. Addresses pvlib#2734.
6d5a9fe to
0239748
Compare
Member
|
@gaoflow in the future please include the PR template checklist when making PRs. I've added it back to this one. |
Contributor
Author
|
Thanks, and apologies for omitting the checklist - I will include the PR template on future PRs. |
echedey-ls
reviewed
Jun 14, 2026
State the -45 to +60 C validity range of the Magnus form directly in the Notes, citing WMO-No. 8 [1], instead of deferring to the reference.
cwhanse
approved these changes
Jun 15, 2026
Comment on lines
+60
to
+61
| :py:func:`pvlib.atmosphere.tdew_from_rh`) so the actual and saturation vapor | ||
| pressures are no longer swapped in the source. The returned values are |
Member
There was a problem hiding this comment.
Suggested change
| :py:func:`pvlib.atmosphere.tdew_from_rh`) so the actual and saturation vapor | |
| pressures are no longer swapped in the source. The returned values are | |
| :py:func:`pvlib.atmosphere.tdew_from_rh`) related to the actual | |
| and saturation vapor pressures. Function output is |
Comment on lines
+368
to
+369
| Relative humidity is computed as ``100 * e / es``, where the actual vapor | ||
| pressure ``e`` is the saturation vapor pressure at the dew point and the |
Member
There was a problem hiding this comment.
Suggested change
| Relative humidity is computed as ``100 * e / es``, where the actual vapor | |
| pressure ``e`` is the saturation vapor pressure at the dew point and the | |
| Relative humidity is computed as ``100 * e / es``, where ``e`` is the | |
| saturation vapor pressure at the dew point temperature, and ``es`` |
Comment on lines
+370
to
+371
| saturation vapor pressure ``es`` is evaluated at the air temperature, both | ||
| from the Magnus equation ``A * exp(B * T / (C + T))``. The default |
Member
There was a problem hiding this comment.
Suggested change
| saturation vapor pressure ``es`` is evaluated at the air temperature, both | |
| from the Magnus equation ``A * exp(B * T / (C + T))``. The default | |
| is the saturation vapor pressureat the air temperature, both | |
| from the Magnus equation ``A * exp(B * T / (C + T))``. The default |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
docs/sphinx/source/referencefor 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`).remote-data) and Milestone are assigned to the Pull Request and linked Issue.This is a clarity change with no change to the returned values. As noted in #2734 and independently confirmed by @echedey-ls (cross-checked against an online calculator and NREL's pvdeg),
rh_from_tdewalready returns correct relative humidity — e.g.rh_from_tdew(20, 10) == 52.56 %,rh_from_tdew(25, 25) == 100 %, and it round-trips withtdew_from_rh. What confused the report is that the two intermediate variables were named backwards:ewas computed fromtemp_air(so it was really the saturation pressure) andesfromtemp_dew(the actual pressure), and100 * es/ewith those swapped names happens to equal the conventional100 * e/es.Changes:
efrom the dew point (actual vapor pressure) andesfrom the air temperature (saturation) so the source matches the usual convention, and write100 * e/es. Numerically identical — the existing value-pinning tests (test_rh_from_tdew,test_tdew_from_rh,test_tdew_to_rh_to_tdew) pass unchanged.tdew_from_rh(it referencedes/e).Notessection and the Alduchov & Eskridge (1996) reference for the Magnus form, since the WMO guide alone doesn't name it (per @echedey-ls).I left the
exp-division micro-optimisation @echedey-ls mentioned out of this first pass since he flagged maintainers may differ on it — happy to add it (or expand the applicability notes with a max-error / valid-range figure) if wanted.