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

Mobt515 cloud base height spot extraction #1911

Merged
merged 13 commits into from
Jun 27, 2023

Conversation

mspelman07
Copy link
Contributor

@mspelman07 mspelman07 commented Jun 12, 2023

Addresses metoppv/mo-blue-team#515
Acceptance_test_data PR: metoppv/improver_test_data#14

Adds an additional plugin and test that adjusts spot data to be relative to site altitude rather than grid square orography.

Testing:

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

@mspelman07 mspelman07 changed the title Mobt515 cbh spot Mobt515 cloud base height spot extraction Jun 12, 2023
@codecov
Copy link

codecov bot commented Jun 13, 2023

Codecov Report

Merging #1911 (8a182b2) into master (cbb9370) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 8a182b2 differs from pull request most recent head 3527de5. Consider uploading reports for the commit 3527de5 to get more accurate results

@@            Coverage Diff             @@
##           master    #1911      +/-   ##
==========================================
+ Coverage   98.33%   98.34%   +0.01%     
==========================================
  Files         120      121       +1     
  Lines       11390    11486      +96     
==========================================
+ Hits        11200    11296      +96     
  Misses        190      190              
Impacted Files Coverage Δ
improver/spotdata/apply_lapse_rate.py 98.38% <100.00%> (ø)
improver/spotdata/height_adjustment.py 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

Copy link
Contributor

@brhooper brhooper left a comment

Choose a reason for hiding this comment

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

Thanks @mspelman07, this looks good, I've just left a couple of very minor comments.

@cli.clizefy
@cli.with_output
def process(
spot_cube: cli.inputcube, neighbour: cli.inputcube,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should neighbour_selection_method be made available as an input in the cli?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it should. I've updated the CLI to include this

# Points outside the range of the original data return NAN. These points are replaced
# with the highest or lowest along the axis depending on the whether the vertical
# displacement was positive or negative
indicies = np.where(np.isnan(spot_data))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
indicies = np.where(np.isnan(spot_data))
indices= np.where(np.isnan(spot_data))

Should also be changed in the next few lines as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. One day I'll learn how to spell indices.

brhooper
brhooper previously approved these changes Jun 19, 2023
Copy link
Contributor

@brhooper brhooper left a 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 @mspelman07, I'm happy with this now

Copy link
Contributor

@gavinevans gavinevans left a comment

Choose a reason for hiding this comment

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

Thanks for adding this functionality @mspelman07 👍

I've added some comments and queries.

improver/cli/apply_height_adjustment.py Outdated Show resolved Hide resolved
improver/spotdata/height_adjustment.py Outdated Show resolved Hide resolved
improver/cli/apply_height_adjustment.py Outdated Show resolved Hide resolved
improver/spotdata/height_adjustment.py Outdated Show resolved Hide resolved
improver/spotdata/height_adjustment.py Outdated Show resolved Hide resolved
improver/spotdata/height_adjustment.py Outdated Show resolved Hide resolved
improver/spotdata/height_adjustment.py Show resolved Hide resolved
improver/spotdata/height_adjustment.py Show resolved Hide resolved
improver/spotdata/height_adjustment.py Show resolved Hide resolved
improver/cli/apply_height_adjustment.py Outdated Show resolved Hide resolved
Copy link
Contributor

@gavinevans gavinevans left a comment

Choose a reason for hiding this comment

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

Thanks @mspelman07 👍

I've added a few minor comments.

improver/cli/apply_height_adjustment.py Outdated Show resolved Hide resolved
improver/spotdata/height_adjustment.py Outdated Show resolved Hide resolved
improver/spotdata/height_adjustment.py Outdated Show resolved Hide resolved
@gavinevans gavinevans merged commit 89fcd30 into metoppv:master Jun 27, 2023
8 checks passed
bayliffe added a commit to bayliffe/improver that referenced this pull request Jul 28, 2023
* upstream/master:
  Fix to the wind vertical displacement adjustment implementation (metoppv#1927)
  Add function which normalises input cubes according to a reference (metoppv#1919)
  Skip ECC bounds usage when converting probabilities to percentiles (metoppv#1926)
  Add CLIs to support rescaling of the forecast based on altitude difference (metoppv#1917)
  Changes to the modal code to increase the percentage to 30% and change the groupings to provide a more representative daily summary symbol. (metoppv#1925)
  Add plugins to support rescaling of the forecast based on altitude difference (metoppv#1916)
  Support conversion from percentiles to probabilities (metoppv#1924)
  Correct handling of reference time in weather_code plugin (metoppv#1920)
  Add CLI for clipping cubes (metoppv#1918)
  Update cbh ecc name (metoppv#1922)
  Updates Broadcast and expand_bounds in Combine Plugin (metoppv#1914)
  Mobt515 cloud base height spot extraction (metoppv#1911)
  MOBT-494: Cube title setting in weather symbol code (metoppv#1912)
  MOBT512-masking percentiles for cloud base height (metoppv#1908)
  Mobt 496 enforce forecast between references (metoppv#1907)
MoseleyS pushed a commit to MoseleyS/improver that referenced this pull request Aug 22, 2024
* Adds plugin to adjust height above ground level forecasts and relevant tests

* corrects handling of NAN data

* formatting and removes un-needed test

* formatting

* formatting

* update docstring

* removes comment

* add neighbour_selection_method to cli

* correct spelling

* formatting

* changes following review

* formatting

* Updates following review comments

---------

Co-authored-by: Marcus Spelman <marcus.spelman@metoffice.gov.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants