-
Notifications
You must be signed in to change notification settings - Fork 145
EN4 cmorizer #4193
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
base: main
Are you sure you want to change the base?
EN4 cmorizer #4193
Conversation
|
Hi, is there someone who could review this PR please? I have a few failing tests so any help would be much appreciated! |
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.
hi @hanellis there seems to be an issue with one of the yaml files, maybe a bit of a bad formatting (looks like an extra empty space), could you please check them? 🍺
Co-authored-by: Valeriu Predoi <valeriu.predoi@gmail.com>
valeriupredoi
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 good, many thanks @hanellis - just one obesrvation about missing tos and sos 🍺
valeriupredoi
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.
very many thanks @hanellis 🍺 One of you good folks could do a scientific review, when you have a minute, please? @axel-lauer @jlenh @LisaBock - cheers!
Sorry, completely missed that! I can have a look at it later today! |
jlenh
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 a lot Hannah, it looks good! I have only a few comments, notably regarding the cmorized variables like V already suggested 🤓 The outputs look good!
On a technical note, I have managed to run the cmorizer but only for a few years on a simple computer, otherwise I get some HDF5 errors. I am in the process of running the whole thing with a computing node on Levante though.
Can you think of a better way to handle the many opened data files here @valeriupredoi maybe? It seems to crash for me when writing the data.
Eventually we will need somebody (@axel-lauer ?) to transfer the data to the OBS data pool on Levante once it is fully processed (I will send you the file paths by email once it's good to go).
jlenh
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.
And forgot the Codacy issues that can be fixed
Co-authored-by: Julien Lenhardt <45034763+jlenh@users.noreply.github.com>
Co-authored-by: Julien Lenhardt <45034763+jlenh@users.noreply.github.com>
|
Thank you @jlenh for picking this up! I've put through the requested changes. I'll have a think if there's a way to address the many data files on my end but please just let me know if there's anything else I need to do/change in the meantime! |
Description
Adds support for EN4 monthly ocean data to ESMValTool.
Currently, ESMValTool lacks monthly 3D observational datasets for ocean temperature and salinity. This addition fills that gap by integrating EN4 monthly data, enabling more comprehensive ocean diagnostics and evaluations.
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: