-
Notifications
You must be signed in to change notification settings - Fork 145
Update ESACCI Cloud CMORizer (daily and monthly data) #3756
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
Conversation
9a8f59b to
7c8eed7
Compare
|
@axel-lauer and @schlunma I hopefully addressed all your comments and tested it again. Works fine for me. Could you have another check on it? Thanks. |
|
Thanks for addressing my comments @LisaBock ! Looks great already, I only have a couple of minor comments left. I think this is almost ready 👍 |
Co-authored-by: Manuel Schlund <[email protected]>
…ESMValTool into update_esacci_cloud_monthly
Thanks again @schlunma ! I tried to address all new comments. The only issue is the switch for downloading and cmorizing daily data. Is there another possibility for adding a switch for the download of daily data?The amount of daily data is so huge that I don't think we should have it as default. |
schlunma
left a comment
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.
Thanks so much Lisa! Approving this from a technical point of view 🚀 I have not run the CMORizer again.
The only issue is the switch for downloading and cmorizing daily data. Is there another possibility for adding a switch for the download of daily data?The amount of daily data is so huge that I don't think we should have it as default.
As I already mentioned in the comment above, there's currently no simple way to achieve this, so I guess it's fine like this.
Thanks!!
axel-lauer
left a comment
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 looks very good and is almost ready to be merged. I suggest to also add the AM and PM datasets to recipe_check_obs.yml (please see suggested changes).
There seems to be, however, a problem with the monthly mean PM dataset. When using the dataset in a recipe (e.g. recipe_check_obs.yml), I get the following error message:
time: Frequency mon does not match input data
I suspect this might be caused by some month missing in the dataset and would need to be checked before merging the PR.
|
Thanks @axel-lauer for reviewing! I also added a function to the formatter for filling in missing months. I had that already, but it got lost somehow in the process. for me, the tests worked fine. |
axel-lauer
left a comment
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.
Looks all good now. Thank you @LisaBock
Description
This PR extends the capabilities of this branch by allowing to download and format daily and monthly ESA CCI Cloud data.
The ESA CCI Cloud website is: ESA CCI Cloud
- Daily Data can be found here.
- Monthly Data can be found here
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.
New or updated data reformatting script
To help with the number of pull requests: