improver icon indicating copy to clipboard operation
improver copied to clipboard

Make Gradient Module Divide by Distance Instead of Degrees

Open MO-PeterJordan opened this issue 1 year ago • 2 comments

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 avatar Mar 13 '24 19:03 MO-PeterJordan

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?

MO-PeterJordan avatar Mar 27 '24 18:03 MO-PeterJordan

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
@@            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.

codecov[bot] avatar Apr 03 '24 09:04 codecov[bot]

In order to maintain a backlog of relevant PRs, we automatically label them as stale after 60 days of inactivity.

If this PR is still important to you, then please comment on this PR and the stale label will be removed.

Otherwise this PR will be automatically closed in 30 days time.

github-actions[bot] avatar Aug 21 '24 01:08 github-actions[bot]