Skip to content

Conversation

@valeriupredoi
Copy link
Contributor

@valeriupredoi valeriupredoi commented Sep 8, 2021

Description

This is a Pull Request to speed up the support for ESACCI LST data and its integration in ESMValCore/Tool, changes belong to @morobking and I have just opened it to streamline the code integration process, I am not an author of the code, just assisting with technical matters.

Review Notes

  • CMIP6_Amon table should NOT change, we will remove the change soon!

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 pull requests:

@valeriupredoi valeriupredoi changed the title ESACCI LST Uncert Adding custom CMOR tables for variables from ESACCI LST Uncert Sep 8, 2021
@valeriupredoi valeriupredoi added the cmor Related to the CMOR standard label Sep 8, 2021
@valeriupredoi valeriupredoi marked this pull request as draft September 8, 2021 09:51
@codecov
Copy link

codecov bot commented Sep 8, 2021

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.47%. Comparing base (444cdbf) to head (ffe4c52).
Report is 69 commits behind head on main.

❗ Current head ffe4c52 differs from pull request most recent head 98f08c3. Consider uploading reports for the commit 98f08c3 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1308   +/-   ##
=======================================
  Coverage   93.47%   93.47%           
=======================================
  Files         238      238           
  Lines       12932    12932           
=======================================
  Hits        12088    12088           
  Misses        844      844           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@valeriupredoi
Copy link
Contributor Author

valeriupredoi commented Sep 8, 2021

@morobking here is the PR for your changes related to the ESACCI LST custom tables; many thanks for your contributions, I have opened this PR just to streamline the integration process, all credit and necessary changes to be made belong to you 😁

A few important points:

  • CMIP6_Amon stock CMOR table should not change; if you think that table needs to change since there is a bug in, please open a separate Pull Request with only that change and assign @jvegasbsc and myself to it; if that change needs to be integrated in the PCMDI/CMOR upstream, we'll fork it there
  • about installing and using this branch in conjunction with the relevant ESMValTool branch (that needs an update with the latest main too, I'll do it in a sec; EDIT it is in fact an entirely different branch, which I have just opened a PR for and updated it with the latest main), please follow the development install instructions: in your usual esmvaltool conda environment, where you have ESMValTool installed:
cd ESMValTool  # go where your esmvaltool source code lives
git checkout ESACCI-LST  # checkout your branch
git pull origin ESACCI-LST  # grab the latest remote branch
pip install -e .[develop]  # install the branch in development mode
cd ~/ESMValCore  # now go where esmvalcore source code lives
git checkout ESACCI_LST_uncerts  # checkout this, latest branch
git pull origin ESACCI_LST_uncerts
pip install -e .[develop]  # install the branch in development mode, overriding any conda installed version of esmvalcore
esmvaltool --version  # check versioning to see you got the latest version installed (core should be 2.3.1 and tool should be 2.3.0)

then you should be ready to test you cmorizer and diagnostic; if you run into any issues please post here, so we can have more than one person looking at any issue 🍺

@morobking
Copy link

morobking commented Sep 21, 2021

Many Thanks @valeriupredoi for putting all this together :)

I'm using the ESMValTool branch CMUG-WP5.4-LST for this work, its a separate work package I'm working under for this but if its easier for you, then I can use the ESACCI-LST branch as this work probably superceeds the previous diagnostic.

When running the command
pip install -e .[develop]
I get this error
ERROR: After October 2020 you may experience errors when installing or updating packages. This is because pip will change the way that it resolves dependency conflicts.

We recommend you use --use-feature=2020-resolver to test your packages with the new resolver before it becomes the default.

scitools-iris 3.1.0 requires cftime>=1.5.0, but you'll have cftime 1.2.1 which is incompatible.
esmvalcore 2.3.1 requires cf-units>=3.0.0, but you'll have cf-units 2.1.4 which is incompatible.
esmvalcore 2.3.1 requires scitools-iris<3.0.4,>=3.0.2, but you'll have scitools-iris 3.1.0 which is incompatible.
Successfully installed ESMValTool esmvalcore-2.3.1 importlib-resources-5.2.2 matplotlib-3.3.4 scitools-iris-3.1.0 zipp-3.5.0

is this still ok?

@valeriupredoi
Copy link
Contributor Author

valeriupredoi commented Sep 22, 2021

@morobking cheers for the update! Not quite OK since you have a rather old version of pip, so I can only imagine that your esmvaltool conda environment is rather old too. Can you please do the following in your conda (base) environment to update it and run the latest tools:

# in (base) environment
conda update -n base conda  # this will update your conda and its base packages
conda install -y -c conda-forge mamba  # grab mamba since it's much faster than conda
mamba env create -n esmvaltool-new -f environment.yml  # create a new and up-to-date environment called esmvaltool-new
conda activate esmvaltool-new  # activate env once it's got built

then you can follow the install setup as above; note that you can just remove your existing esmvaltool environment and create a new one with the same name, if you don't want to create one with a different name eg esmvaltool-new. Let me know if you have any problems 👍

Copy link
Contributor

@axel-lauer axel-lauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The custom tables look technically fine, but I was wondering why there are pairs of tables containing the same information only with "Day" added to the variable name, e.g.:

tsLSSysErr and tsLSSysErrDay

If this is to support different temporal resolutions (monthly means, daily means), it should theoretically work to have only one table (in the example above tsLSSysErr) and specify the temporal resolution in the recipe (variable definition), e.g. mip: Amon or mip: Day

As @valeriupredoi already pointed out, esmvalcore/cmor/tables/cmip6/Tables/CMIP6_Amon.json should (if possible) not be changed (also no additions). This is what custom CMOR tables are used for.

@morobking
Copy link

Running this command:
mamba env create -n esmvaltool-new -f environment.yml
I get this message:
SpecNotFound: Invalid name, try the format: user/package

I've tried adding my user name and a / in a few place to see if thats what user/package means but havent been able to find something that works.
I've not come across mamba before so not sure what I'm looking for.

@zklaus
Copy link

zklaus commented Sep 27, 2021

That error message indicates an old(ish) version of mamba. Mamba only recently gained the ability to handle -n and -f in the creation of a new environment at the same time. Could you update mamba and see how it goes?

"frequency": "mon",
"modeling_realm": "atmos",
"standard_name": "surface_temperature",
"units": "K",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not change anything in the CMIP6 tables if at all possible. We may replace them with a newer version and all changes will be lost

@morobking
Copy link

thank @zklaus
I've run
conda update mamba
and then rerun the mamba command with the same error
mamba -V
gives:
conda 4.10.3

@valeriupredoi
Copy link
Contributor Author

@morobking that's because when you do mamba -V it always prints the conda version whose mum it is. You can find out what version you have by doing conda list mamba in the environment where mamba is installed (preferably the base environment). Anyway, for your issue - are you running the env creation command in the ESMValCore directory where environment.yml lives? If I run it in a dir where there is no environment.yml I get exactly the same error you get:

(base) valeriu@valeriu-PORTEGE-Z30-C:~$ mamba env create -n dfr -f environment.yml

SpecNotFound: Invalid name, try the format: user/package

@morobking
Copy link

Thanks all
Setting up Linux environments is not my natural habitat!

I ran the conda stuff in my ESMValTool directory as there was an environment.yml file there.
This has run smoothly and by the screen outputs, done a lot of things.

I've gone back to @valeriupredoi original 'install' instructions and now get
esmvaltool version
ESMValCore: 2.3.1
ESMValTool: 2.3.0

@valeriupredoi
Copy link
Contributor Author

@morobking excellent work, you're in business 💼

@morobking
Copy link

I've just run the cmorizor as it stands and it has successfully done what I thought it would do!
If I've understood everything correct, I can move questions etc.. to the the second PR @valeriupredoi set up ESMValGroup/ESMValTool#2291

Thanks all

@schlunma

This comment was marked as off-topic.

@schlunma schlunma mentioned this pull request Mar 10, 2022
10 tasks
…h tsDay and tsNight still. CMORized files give esmvalcore.cmor.check.CMORCheckError: There were errors in variable lst:clst: does not match coordinate rank
schlunma and others added 23 commits January 23, 2024 12:32
Co-authored-by: Valeriu Predoi <[email protected]>
@morobking
Copy link

@axel-lauer I've committed updated CMOR tables for the CCI LST data. I'm just tidying the ESMValTool branches that use them for cmorizing and loading the data in a recipe.
I'm not sure where the short_name that is being assigned to the files the CMORizer creates is coming from. Maybe that will be clearer when I make a commit on ESMValTool branch?
The short names start lst_ which as far as I can find, is not something I have added here.

@github-actions
Copy link
Contributor

In order to maintain a backlog of relevant pull requests, we automatically label them as stale after 180 days of inactivity.

If this pull request is still important to you, please comment below to remove the stale label. Otherwise, this pull request will be automatically closed in 60 days. If this pull request only suffers from a lack of reviewers, please tag the @ESMValGroup/technical-lead-development-team so they can help you find a suitable reviewer.

@github-actions github-actions bot added the Stale label Jun 26, 2025
@morobking
Copy link

This is not stale - paused this while myself, @axel-lauer and CLaire Bulgin reviewed the maths/science in the related PRs and code.

@github-actions github-actions bot removed the Stale label Jun 28, 2025
@morobking
Copy link

@axel-lauer @valeriupredoi I believe this branch is ready to finish the PR. It contains new .dat files for the LST Uncertainty variables. My understanding is everything needed for the whole work package will be in separate the ESMValTool PR which I'll resurrect once this gets going.
These are needed for the CMORizor in ESMValTool branch cci_lst_v3_uncert_diagnostic which has cmorizor and diagnostic which I'll update shortly with the final diagnostic code after the scientific work earlier this year.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cmor Related to the CMOR standard orphan PR that has been forgotten and now needs serious dusting off

Projects

None yet

Development

Successfully merging this pull request may close these issues.