Allowed arbitrary CMIP6 fx variables in special preprocessors and added a catch on project not found in config-developer (invalid project)

[schlunma]

Tasks

• Create an issue to discuss what you are going to do, if you haven't done so already (and add the link at the bottom)
• This pull request has a descriptive title that can be used in a changelog
• Add unit tests
• Public functions should have a numpy-style docstring so they appear properly in the API documentation. For all other functions a one line docstring is sufficient.
• If writing a new/modified preprocessor function, please update the documentation
• Circle/CI tests pass. Status can be seen below your pull request. If the tests are failing, click the link to find out why.
• Codacy code quality checks pass. Status can be seen below your pull request. If there is an error, click the link to find out why. If you suspect Codacy may be wrong, please ask by commenting.
• Please use yamllint to check that your YAML files do not contain mistakes
• If you make backward incompatible changes to the recipe format, make a new pull request in the ESMValTool repository and add the link below

If you need help with any of the tasks above, please do not hesitate to ask by commenting in the issue or pull request.

---

Closes #431.

[valeriupredoi]

very cool @schlunma 🍺 Note the fix for various mips here #429 -> we should merge that one first then this one

[schlunma]

I think we can tackle the #429 in a more general way here. Can you test if #405 is fixed in this branch @ledm? I have to add tests for this.

[valeriupredoi]

Yeh nicely generalized in the new framework, Manu! The question is - do we really want to raise the not found fx files? I'd be cautious about that since the fx var might not simply be there and NE masks will be used instead 🍺

[schlunma]

We only raise an error if the fx var is not in the CMOR tables, if only the files are not available no error is raised.

[valeriupredoi]

Ow true, my bad, then I'd move the check up where the cmor variable is built, use a try/except with a debug logger mssg, and let the code go through even if the cmor variable is not built - if no cmor variable found return an empty fx_files list so NE masks can still be used

[valeriupredoi]

Actually, no need for try/except block, just remove the searched_mips and its use and referencing Dr Valeriu Predoi. Computational scientist NCAS-CMS University of Reading Department of Meteorology Reading RG6 6BB United Kingdom

…

On Thu, 16 Jan 2020, 09:37 Valeriu Predoi, ***@***.***> wrote: Ow true, my bad, then I'd move the check up where the cmor variable is built, use a try/except with a debug logger mssg, and let the code go through even if the cmor variable is not built - if no cmor variable found return an empty fx_files list so NE masks can still be used Dr Valeriu Predoi. Computational scientist NCAS-CMS University of Reading Department of Meteorology Reading RG6 6BB United Kingdom On Thu, 16 Jan 2020, 09:27 Manuel Schlund, ***@***.***> wrote: > We only raise an error if the fx var is not in the CMOR tables, if only > the files are not available no error is raised. > > — > You are receiving this because your review was requested. > Reply to this email directly, view it on GitHub > <#432?email_source=notifications&email_token=AG5EFIYOJQ24GPNTAOP7NITQ6ASATA5CNFSM4KHFLTVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJDL72Q#issuecomment-575061994>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AG5EFI5RIES5O3FLMOKITHLQ6ASATANCNFSM4KHFLTVA> > . >

[schlunma]

But why would we need/allow an fx variable name that is not in the CMOR tables? E.g. if you have a typo in your fx name, then it is really hard to figure out what is failing.

EDIT: right now, this is only done for CMIP6

[valeriupredoi]

Hmm, that might be. Am just concerned fx_mips is not exhaustive enough for hells knows what corner cases. But I guess there it's better to actually raise a big red flag anyway, rather then quietly apply NE masks

[valeriupredoi]

Also I believe @ledm case of volcello found in Omon rather than Ofx will not work now since fx_var can not be constructed with Omon so the file search will be done with Ofx instead, and return empty

[schlunma]

Also I believe @ledm case of volcello found in Omon rather than Ofx will not work now since fx_var can not be constructed with Omon so the file search will be done with Ofx instead, and return empty

I'm pretty sure it works, this line should help:

ESMValCore/esmvalcore/_recipe.py

Line 398 in 6719ca4

fx_mips.append(variable['mip'])

I also added a test for it which should cover this case

[schlunma]

volcello is also in the table Omon, so fx_variable builds just fine 😉

[ledm]

Does this fix address the other problem raised in #405: Some parts of the preprocessor chain need to be applied to the fx file as well as the diagnostic variable data.

[valeriupredoi]

Nope, that's a completely different dish of curry. I'll think about that today.

Also, apologies for my previous email comments introducing my lengthy email signature 🍺

[valeriupredoi]

@schlunma I have compacted your implementation to serve irrespective of project and added a couple more tests; I am changing the Approved status (that I gave yesterday) so that you and @ledm can test 🍺

[valeriupredoi]

BTW if I didn't say it - really nice approach @schlunma 🍺

[schlunma]

BTW if I didn't say it - really nice approach @schlunma beer

Thanks 😁

I was a bit scared of expanding this to all projects, but actually this should be no problem.

One thing I mentioned: Do we check if there's a CMOR table for the project somewhere before this code? If not, then cmor_table = CMOR_TABLES[var_project] fails with a really nasty KeyError.

[valeriupredoi]

variable (or var as in our function) is already initialized and _add_cmor_info has been run on it so CMOR_TABLES[var_project] should always return the correct cmor table, man 🍺

[valeriupredoi]

here's a test running with a fictitious CMIP7 as variable["project"]:

esmvalcore/_recipe.py:757: in _get_single_preprocessor_task
    config_user=config_user,
esmvalcore/_data_finder.py:270: in get_output_file
    cfg = get_project_config(variable['project'])
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

project = 'CMIP7'

    def get_project_config(project):
        """Get developer-configuration for project."""
        logger.debug("Retrieving %s configuration", project)
>       return CFG[project]
E       KeyError: 'CMIP7'

esmvalcore/_config.py:171: KeyError

[schlunma]

Nice, so not our business 😄

[valeriupredoi]

of course it is, seaman Beaumont! (ref to Hunt for Red October)
Here's the catch and controlled fail 👍 🍺

[valeriupredoi]

BTW the last test I added is a message for future us when we will run ESMValTool on CMIP7 data 🤣

[bouweandela]

Looks good to me. @mattiarighi Could you please test?