-
Notifications
You must be signed in to change notification settings - Fork 39
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
Added EMAC CMORizer #1554
Added EMAC CMORizer #1554
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1554 +/- ##
==========================================
+ Coverage 90.98% 91.23% +0.25%
==========================================
Files 201 203 +2
Lines 10680 10978 +298
==========================================
+ Hits 9717 10016 +299
+ Misses 963 962 -1
Continue to review full report at Codecov.
|
what's worse - the |
scratch that! am a dummer - it's sequential |
Not suggesting that you must do this change here and now, but mid- to long-term we should replace all of these |
aye, @bouweandela is right - no sign of the emac.nc file in the (correct) test data root:
|
The problem can indeed be fixed by installing git and ssh before checkout on CircleCI. See #1601 for more info. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked through the plots produced by the EMAC example recipe recipe_emac_new.yml
and most things look good - as far as I can tell. There are, however, a few items that might need to be double-checked:
- map of precip (given in kg/m2/s): the color scale ranges from 0-10, i.e. the plot is empty and evaluation is not possible.
- map of zg at 500 hPa: the values over Antarctica and over Greenland look very strange, I would expect the values to range somewhere between 5000 and 6000 m but not between 0 and 3000 m as in the plot.
- time series of surface pressure: assuming that the values shown are global mean values, the absolute values and the variability look wrong
Thanks Axel for having a look at this!!
Ah, yes, apparently the monitoring diagnostics expects units of
That's also present in the raw data (see below). Note that the CMORizer only divides this data by
I think the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I successfully ran recipe_emac_new.yml
and the output looks fine to me. Three quick answers to my previous comments:
- pr: OK, the preprocessor output looks fine, but I am not sure why the diagnostic expects mm/day instead of simply using the units provided in the metadata. But that's not a problem of the EMAC related additions.
- ps: also OK, the preprocessor output looks fine but I do not fully understand why the diagnostic creates such a strange looking time series. But again, this is not related to the EMAC extensions and thus not relevant here.
- zg @ 500 hPa: OK, the original EMAC output already looks weird with the 500 hPa geopotential heitght in Antarctica and Greenland being lower than the surface height! We should ask our EMAC modelers about this, but again, this is not related to the EMAC additions and thus fine.
So in summary, all fine and green light from the science review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR looks good to me: code, documentation, tests, recipe run with recipe_emac_new.yml
. Thanks for your work @schlunma! It's nice to have support for EMAC data as part of the on-the-fly cmorizers.
Awesome, great!! Many thanks for the quick review @axel-lauer @remi-kazeroni 🎉 |
Description
This PR adds a CMORizer for native EMAC model output.
Changes not directly linked to EMAC
load
preprocessor step in theconfig-developer.yml
file, e.g.,ESMValCore/esmvalcore/_recipe.py
Lines 727 to 734 in ccb53b8
and
ESMValCore/esmvalcore/preprocessor/_io.py
Lines 113 to 138 in ccb53b8
Closes #312
Closes #309
Closes #139
Closes #66
Related PRs:
hfns
#1594Link to documentation:
Before you get started
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.