Skip to content
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

Update humidity mixing ratio to work with pressure and height level data #2056

Merged

Conversation

mspelman07
Copy link
Contributor

Addresses https://metoffice.atlassian.net/browse/EPPT-1980

This PR updates the HumidityMixingRatio plugin to work with data on pressure levels or height levels.

As we are moving to using Dag runner and won't need a CLI I haven't included any acceptance tests or CLI's. This might need to be added later on after wider discussions with IMPROVER team and others about the working practices going forward.

Testing:

  • Ran tests and they passed OK
  • Added new tests for the new feature(s)

@mspelman07 mspelman07 added the EPP PR's related to Enhancing Post Processing team label Nov 27, 2024
Copy link

codecov bot commented Nov 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.38%. Comparing base (84a8944) to head (39a9c61).
Report is 46 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2056      +/-   ##
==========================================
- Coverage   98.39%   98.38%   -0.01%     
==========================================
  Files         124      133       +9     
  Lines       12212    13087     +875     
==========================================
+ Hits        12016    12876     +860     
- Misses        196      211      +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mo-philrelton
mo-philrelton previously approved these changes Dec 3, 2024
Copy link
Contributor

@mo-philrelton mo-philrelton left a comment

Choose a reason for hiding this comment

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

This looks good to me. All the tests are passing and it seems to be doing what it's supposed to be doing. I let a comment on what looked like a confusing section of code. Please feel free to ignore my suggestion as it's such a minor style change. You have a comment explaining it anyway.

self.pressure = cubes.extract_cube("surface_air_pressure")
except ConstraintMismatchError:
# If no pressure cube is provided, check if pressure is a coordinate in the temperature and relative humidity cubes
temp_coord = [
Copy link
Contributor

Choose a reason for hiding this comment

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

This section seemed a little confusing to me. This is almost exactly the same thing but might be easier to understand/a tiny tiny bit faster:

temp_coord_flag = any(
    coord.name() == "pressure"
    for coord in self.temperature.coords()
)
rh_coord_flag = any(
    coord.name() == "pressure"
    for coord in self.rel_humidity.coords()
)
if temp_coord_flag & rh_coord_flag:
    self.generate_pressure_cube()
else:
    raise ValueError(
        "No pressure cube called 'surface_air_pressure' found and no pressure coordinate found in temperature or relative humidity cubes"
        )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer your code to mine so I've updated it.


Args:
cubes:
Cubes of temperature (K), pressure (Pa) and relative humidity (1)
Cubes of temperature (K) and relative humidity (1). A cube of pressure (Pa) must also
br provided unless there is a pressure coordinate in the temperature and relative humidity cubes.

Choose a reason for hiding this comment

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

small typo here "br" replace with be

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected

@mspelman07 mspelman07 merged commit 566ada8 into metoppv:master Dec 9, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EPP PR's related to Enhancing Post Processing team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants