ESMValGroup / ESMValTool

ESMValTool: A community diagnostic and performance metrics tool for routine evaluation of Earth system models in CMIP
https://www.esmvaltool.org
Apache License 2.0
210 stars 122 forks source link

Validate RTW user config file #3615

Closed chrisbillowsMO closed 1 month ago

chrisbillowsMO commented 1 month ago

Description


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.


To help with the number of pull requests:

chrisbillowsMO commented 1 month ago

Wow - great stuff @chrisbillowsMO! Clearly written and a nice choice of tests.

I've tested on Jasmin and it runs cleanly.

Hi @alistairsellar - great! ๐Ÿ˜ I'm so pleased you're pleased. @ehogan didn't want to miss out on all the fun so I'll await her final feedback and merge once she bequeaths her seal.

As an aside, I've warmed to @ehogan's suggestion of an esmvaltool config validate command: #3392 (comment). But makes sense I think to merge this to RTW now, and then discuss moving the functions to Core for a new command.

Great! I was working with this in mind with a view to having a crack at the issue raised against it in ESMValCore:

One point to consider: it may be a bigger job to implement this in core.

CFG/Configure() objects are specifically designed to throw an error on the first invalid value. I had to write a workaround to collect multiple errors (i.e. to save the user multiple runs/failures).

I wrote it up a little here: https://github.com/ESMValGroup/ESMValTool/issues/3392#issuecomment-2124795203 but the tldr; for core we'd probably want a different, more holistic solution than what I've written. Indeed, it might mean re-shaping a lot of the validation code in ESMValCore - probably not ideal! A decorator might offer a solution but I need to fully understand the validation code first. I'm hoping to continue exploring when/if time allows.

Also, you're probably already aware, but possibly to bear in mind are the broader changes afoot in the user config file arena: