-
Notifications
You must be signed in to change notification settings - Fork 1
Get CMEW to work with new-style ESMValTool configuration files #324
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?
Get CMEW to work with new-style ESMValTool configuration files #324
Conversation
…ol-configuration-files
mo-nikosbaltas
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.
run_recipe_radiation_budget failed.
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 tried to debug and I think the changes you made are incorrect, unless I am using an old version of ESMValTool.
In the CMEW/app/run_recipe/rose-app.conf , you stopped passing the CMEW-generated user config to ESMValTool and instead you set the env ESMVALTOOL_CONFIG_DIR
As a result, ESMValTool falls back to its default configuration search behaviour. The job.err proves that:
Reading configuration files from:
.../esmvalcore/config/.../defaults (defaults)
/home/users/nikolaos.baltas/.esmvaltool/config-user.yml (single configuration file
So ESMValTool is not reading the config you generated under:
/home/users/nikolaos.baltas/cylc-run/CMEW/run19/share/etc/user_config/config-user.yml
More detailed job.err extract.
2026-01-12 21:11:29,612 UTC [2912639] INFO Reading configuration files from:
/data/apps/sss/environments/esmvaltool-2.12.0/lib/python3.12/site-packages/esmvalcore/config/configurations/defaults (defaults)
/home/users/nikolaos.baltas/.esmvaltool/config-user.yml (single configuration file [deprecated])
2026-01-12 21:11:29,612 UTC [2912639] INFO Writing program log files to:
/home/users/nikolaos.baltas/esmvaltool_output/recipe_radiation_budget_20260112_211129/run/main_log.txt
/home/users/nikolaos.baltas/esmvaltool_output/recipe_radiation_budget_20260112_211129/run/main_log_debug.txt
/home/users/nikolaos.baltas/esmvaltool_output/recipe_radiation_budget_20260112_211129/run/cmor_log.txt
The question is whether EsmValTool 2.12.0 supports ESMVALTOOL_CONFIG_DIR. It does not seem to. Do we need to use a different version?
|
For me it is running! I get this in the logs: So I'll see if I can work out why it isn't for you. Does it still happen if you don't load the esmvaltool community module independently @mo-nikosbaltas, and just |
|
I tried on a clean terminal using your PR branch, and still got the same error.
|
|
Further investigation on the failure I was getting. |
|
@NParsonsMO check #336 for further explanation. Also in: This line I made the changes myself and the runs succeeded. From the logs I can confirm that we are now using CMEW's configuration. Plots were created: |
mo-nikosbaltas
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.
See comments above. I run the pr-324 branch with the changes mentioned.
|
Thanks for investigating the problem @mo-nikosbaltas! 🎉
As described in #294 (comment), the
This was needed in the old style configuration file. I expect this is the reason why the directory provided by the |
Mine is not at However, I was under the impression (from the conversation with @ehogan after stand-up a few days ago) that we were not meant to use |
|
@mo-nikosbaltas or @ehogan , it would be really good if at least one of you could double check my last commit removing the test, |
mo-nikosbaltas
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.
All looks good. Tests passed. Approved.
ehogan
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 @NParsonsMO 🥳
| # Write the updated configuration values to the file defined by | ||
| # 'USER_CONFIG_PATH'. | ||
| user_config_path = values["USER_CONFIG_PATH"] | ||
| os.makedirs(os.path.dirname(user_config_path), exist_ok=True) |
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 is not necessary. The directory is made by the configure_recipe/rose-app.conf file.
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.
If I literally comment out the line, then the run fails.
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.
Conclusion:
Once the changes below were implemented then this line could be removed.
CMEW/flow.cylc
Outdated
| ESMVALTOOL_CONFIG_DIR = ${USER_CONFIG_DIR}/user_config | ||
| USER_CONFIG_PATH = ${ESMVALTOOL_CONFIG_DIR}/config-user.yml |
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.
Thinking about how the configuration now works in ESMValTool, I wonder whether we need to drop the term USER_CONFIG_DIR, since it's now one big configuration directory (USER_CONFIG_PATH is ok for now to reference the file itself, but that might change in the future when we look at the formatting / contents of that file).
Using ESMVALTOOL_CONFIG_DIR in favour of USER_CONFIG_DIR would minimise the use of variables here, and should solve the directory creation issue (as mentioned in my previous 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.
|
Following conversation this morning, I have tidied the directories to the following structure within Note that I haven't also moved |
mo-nikosbaltas
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.
Failed unittest: cylc vip -O metoffice -O unittest
The [9 remaining] unit tests run fine for me locally with Maybe @ehogan can help?! |
|
When I run : |
The error when running This error occurs because the To solve this, we could move the creation of the |
I think the creation of this directory is really part of |
Great question! 🥳 The traceback shows that ESMValTool is checking that the directory defined by |
|
I just had a thought; is it because |
We need in available to the run_recipe task, to solve the main issue. I tried define it (with and without the associated Do you want me to keep looking, or are you happy to create it in the install_env task, even though it may not be obvious in future why it's there? Or, alternatively, have the |
ehogan
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.
Almost there! 🥳
| "input_key, output_key, expected", | ||
| [ | ||
| (None, "remove_preproc_dir", False), | ||
| ("USER_CONFIG_PATH", "config_file", "userpath"), |
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.
The copyright in this file needs updating 😊
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.
Oops, done.





Closes #294.
PR creation checklist for the developer
<issue_number>above ☝️ been replaced with the issue number?mainbeen selected as the base branch?<issue_number>_<short_description_of_feature>?good first issuelabel) been added to the PR?Climate Model Evaluation Workflow (CMEW)project been added to the PR?Definition of Done for the developer
docdirectory) related to the change been updated appropriately, including the Quick Start section?PR creation checklist for the reviewer
<issue_number>above ☝️ been replaced with the issue number?mainbeen selected as the base branch?<issue_number>_<short_description_of_feature>?good first issuelabel) been added to the PR?Climate Model Evaluation Workflow (CMEW)project been added to the PR?Definition of Done for the reviewer
docdirectory) related to the change been updated appropriately, including the Quick Start section? N/A