Fix transposed Hellmann exponent table in windspeed_powerlaw docstring - #2853
Fix transposed Hellmann exponent table in windspeed_powerlaw docstring#2853dylanpulver wants to merge 2 commits into
Conversation
The docstring table lists stability down the rows and surface type across the columns, but the nine values were laid out in the order they appear in HELLMANN_SURFACE_EXPONENTS, which groups by surface type. Four of the nine cells therefore disagree with the dict the function actually uses. The corrected table is the transpose of the printed one, which is why the five cells on the diagonal already agreed.
Hey @dylanpulver! 🎉Thanks for opening your first pull request! We appreciate your If AI is used for any portion of this PR, you must vet the content |
|
There's no freaking way you've opened 107 PRs in September by 2nd Sept. Closing as AI slop solution, I doubt you really checked the source papers and the suggested changes. The underlying issue is real thou, a mismatch between the table coeffs and the dict coeffs. @IoannisSifnaios can you have a look at it? I may open an issue later. |
|
I'm going to re-open this one. Its the kind of error that AI excels at catching. I've checked the reference, the PR is correct. AI is like gravity, we can fight it, or let it work for us. |
|
We'll see if @dylanpulver agents address the editorial suggestion I made. |
adi-IL
left a comment
adi-IL
left a comment
There was a problem hiding this comment.
Checked against HELLMANN_SURFACE_EXPONENTS in pvlib/atmosphere.py and Kaltschmitt et al. (p. 55).
The table rows are stability states (unstable, neutral, stable) while the columns are roughness categories (open water, coast, inhabited). The four off-diagonal cells previously inverted the surface and stability axes:
- Unstable over coast: 0.11 (was 0.10)
- Neutral over open water: 0.10 (was 0.11)
- Neutral over inhabited: 0.34 (was 0.40)
- Stable over coast: 0.40 (was 0.34)
The corrected matrix reconciles the docstring table with HELLMANN_SURFACE_EXPONENTS across all nine entries.
4 participants
The Hellmann exponent table in the
windspeed_powerlawdocstring is the transpose ofHELLMANN_SURFACE_EXPONENTS, the dict the function actually uses. The nine values were written out in dict order, which groups by surface type, while the table's rows are stability - so the five cells on the diagonal agree and the four off-diagonal ones are swapped in pairs: 0.10 and 0.11 between open water and coast, 0.34 and 0.40 between coast and cities.The dict is the right one. Kaltschmitt, Streicher and Wiese (reference [1] in this docstring, p. 55) give unstable/neutral/stable = 0.06/0.10/0.27 over open water, 0.11/0.16/0.40 over flat open coast, 0.27/0.34/0.60 over inhabited areas - exactly the dict, whose key names are that source's row labels. Docstring only, no behaviour change.
Nothing exercised the disagreement: neither
windspeed_powerlawnor the dict has a caller insidepvlib/, and both tests passunstable_air_above_open_water_surface(0.06), one of the cells a transposition leaves alone. Table and dict were added in the same commit, #2124.Verified by parsing the dict and the RST table back out of
atmosphere.pyand comparing all nine cells: 4 mismatches before, 0 after.Whatsnew entry goes in v0.16.0 - happy to move it to v0.15.3 if this should ship in the patch release.
AI disclosure: drafted with Claude Opus 5 (
claude-opus-5); the values were checked against the cited reference.