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

Removed automatic conversion of precipitation units from monitoring diagnostic #3049

Merged
merged 2 commits into from
Mar 1, 2023

Conversation

schlunma
Copy link
Contributor

@schlunma schlunma commented Feb 22, 2023

Description

Currently, the diagnostic monitor/multi_datasets.py automatically converts precipitation units from kg m-2 s-1 to mm day-1 even if the user does not ask for this. The reason for this was that ESMValCore was not able to do that conversion in the preprocessor.

Since ESMValGroup/ESMValCore#1574 and ESMValGroup/ESMValCore#1837 this conversion is now possible; thus, the conversion can be removed from the diagnostic.

How to change recipes?

To use the unit conversion for precipitation in the new version of this diagnostic, add the following preprocessor for the precipitation dataset to the recipe:

convert_units:
  units: mm day-1

Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.

New or updated recipe/diagnostic


To help with the number of pull requests:

@schlunma
Copy link
Contributor Author

To use the unit conversion in the new version of this diagnostic, add the following preprocessor for the precipitation dataset to the recipe:

convert_units:
  units: mm day-1

@schlunma
Copy link
Contributor Author

@esmvalbot Please run monitor/recipe_monitor_with_refs.yml

@esmvalbot
Copy link

esmvalbot bot commented Feb 22, 2023

Since @schlunma asked, ESMValBot will run recipe monitor/recipe_monitor_with_refs.yml as soon as possible, output will be generated here

@esmvalbot
Copy link

esmvalbot bot commented Feb 22, 2023

ESMValBot is happy to report recipe monitor/recipe_monitor_with_refs.yml ran OK, output has been generated here

Copy link
Contributor

@valeriupredoi valeriupredoi left a comment

Choose a reason for hiding this comment

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

do you not want to add the conversion preprocessor to the recipe? Apart from that - one of the easiest reviews ever 😁

@schlunma
Copy link
Contributor Author

do you not want to add the conversion preprocessor to the recipe? Apart from that - one of the easiest reviews ever grin

The recipe uses temperature and not precipitation - so no, I don't want to add that 😅

@valeriupredoi
Copy link
Contributor

surely some American will complain their temperature is indeed measured in mm/day 🤣

@valeriupredoi
Copy link
Contributor

@remi-kazeroni you wanna merge this, bud? Unless you think temperature is measured in mm/day 😆

@remi-kazeroni
Copy link
Contributor

@schlunma, could you please move or duplicate these instructions to the description PR? This would help the release manager to draft the changelog 🍻 This can then be merged. Thanks!

@schlunma
Copy link
Contributor Author

Done!

@remi-kazeroni
Copy link
Contributor

Thanks for your contribution @schlunma and the review @valeriupredoi!

@remi-kazeroni remi-kazeroni changed the title Removed automatic conversion of precipitation units from monitoring diagnoistic Removed automatic conversion of precipitation units from monitoring diagnostic Mar 1, 2023
@remi-kazeroni remi-kazeroni merged commit 93083e9 into main Mar 1, 2023
@remi-kazeroni remi-kazeroni deleted the optimize_monitor_diag branch March 1, 2023 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants