Clarify rh_from_tdew variable naming, comments and references#2782
Open
gaoflow wants to merge 1 commit into
Open
Clarify rh_from_tdew variable naming, comments and references#2782gaoflow wants to merge 1 commit into
gaoflow wants to merge 1 commit 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.
gaoflow
force-pushed
the
fix-2734-rh-from-tdew-naming
branch
from
6d5a9fe to
0239748
Compare
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
1 participant
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.
Addresses #2734.
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.