-
Notifications
You must be signed in to change notification settings - Fork 100
Total Precipitable Water Plugin- EPPT 2571 #2190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
mo-philrelton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks pretty good and produces correct looking data, but there are a few code improvments I think you should make.
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
mo-philrelton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making those changes. I just have a couple more suggestions for you
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
|
thank you for the comments @mo-philrelton I have made these changes. |
mo-philrelton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
MoseleyS
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Phoebe. As discussed, there are a few things that I think could be simplified in this code which will make maintaining it in the future easier.
I also expected to see one or more unit tests testing that the inputs and outputs have specific data values, to help us catch unexpected consequences of future changes. Please can you add these.
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
...r_tests/psychrometric_calculations/total_precipitable_water/test_total_precipitable_water.py
Show resolved
Hide resolved
improver_tests/psychrometric_calculations/precipitable_water/test_precipitable_water.py
Show resolved
Hide resolved
mo-robert-purvis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The plugin interface works as expected - used in EPP workflow for TPW. The resulting cube is successfully manipulated for creating summed TPW. Tests look good.
MoseleyS
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Phoebe. This is doing the right thing. My suggestions are now around making the code easier to understand.
improver_tests/psychrometric_calculations/precipitable_water/test_precipitable_water.py
Show resolved
Hide resolved
...r_tests/psychrometric_calculations/total_precipitable_water/test_total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
improver/psychrometric_calculations/total_precipitable_water.py
Outdated
Show resolved
Hide resolved
|
Thank you for the review @MoseleyS I have made the suggested changes. |
MoseleyS
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests pass (on a branch simulating merging this PR into master).
Anzerkhan27
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests verify the logic, units, and metadata.
MoseleyS
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-approving.
I have checked with Ben A and Carwyn P and we can safely ignore the missing codecov tests.
|
Bypassed rules in this case due to issues with the codecov currently. |
* upstream/master: Alter site filtering for site EMOS (metoppv#2201) Remove Anzer's name due to git merging behaviour. (metoppv#2212) Total Precipitable Water Plugin- EPPT 2571 (metoppv#2190)

Addresses- EPPT-2571
Description
Create a plugin in IMPROVER to calculate precipitable water on pressure levels so it can be used it in the revised workflow design to calculate total precipitable water from the available StaGE data.
Plugin inputs: Humidity Mixing Ratio on pressure levels
Output: Precipitable water on pressure levels
Calculation:
The precipitable water on each pressure level is a function of the pressure difference across the vertical level and mean humidity mixing ratio in each level. We will assume that these values are represented by the input.
The pressure difference will be calculated from the linear mid-point between levels in Pascals. These values will form the bounds of the pressure coordinate on the output.
PW: Precipitable Water (m). q: Humidity mixing ratio (kg kg-1). ∆ p: Pressure difference (Pa == N m-2 == kg m-1 s-2). g: gravitational constant (9.91 m s-2). ρ: Water density constant (1000 kg m-3).
Testing: