metoppv / improver

IMPROVER is a library of algorithms for meteorological post-processing.
http://improver.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
101 stars 84 forks source link

Make Gradient Module Divide by Distance Instead of Degrees #1985

Open MO-PeterJordan opened 4 months ago

MO-PeterJordan commented 4 months ago

The existing gradient module does not divide through by the distance between grid points. Instead it divides through by the angular separation, resulting in units of original-units deg^-1. This PR changes that to divide through by distance in meters, so that the resulting units are original-units m-1. This is what we would naturally expect, and allows eg. relative vorticity to be calculated with the correct units of s^-1.

Potential issues: When calculating the gradient of wind speed (m/s) Iris presents the resulting units as Hz, which might be confusing for a gradient. However, grad_cube.units == s^1 returns true, so iris' units logic for downstream applications will be fine.

Absolute precision requirement has been reduced in the tests from e-8 to e-4 as this threshold was being breached in cases with very large gradients (>6 orders of magnitude between adjacent grid cells) where interpolation where required to recover the original grid shape . This seems to be to do with the way iris does linear interpolation. However O(10^6) gradients between adjacent cells seem unlikely to occur for meteorological parameters. Furthermore, +/-0.0001 should be enough precision for gradients of most if not all parameters.

MO-PeterJordan commented 3 months ago

I think the CLI should be a separate PR, there's plenty on this one already! I don't think I'd actually need one for relative vorticity, as that will call the gradient module, which itself calls the distance module; however, it might be standard practice that all new modules get CLIs?

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 96.29630% with 3 lines in your changes missing coverage. Please review.

Project coverage is 98.37%. Comparing base (84a8944) to head (669c49b). Report is 14 commits behind head on master.

:exclamation: Current head 669c49b differs from pull request most recent head 3fc82f6

Please upload reports for the commit 3fc82f6 to get more accurate results.

Files Patch % Lines
improver/utilities/spatial.py 96.29% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1985 +/- ## ========================================== - Coverage 98.39% 98.37% -0.02% ========================================== Files 124 124 Lines 12212 12276 +64 ========================================== + Hits 12016 12077 +61 - Misses 196 199 +3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.