Skip to content
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

Add functional Autoassess diagnostics: land surface metrics: permafrost, soil moisture, surface radiation #2170

Merged
merged 55 commits into from Jun 17, 2021

Conversation

valeriupredoi
Copy link
Contributor

@valeriupredoi valeriupredoi commented May 12, 2021

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.

New or updated recipe/diagnostic

New or updated data reformatting script


To help with the number of pull requests:

Autoassess landsurface metrics/diagnostics

Battle plan to include the Autassess landsurface recipes in ESMValTool v2.3:

  • resurrect them from the now extremely old all_autoassess_recipes branch into a new branch (this one) and create Draft PR
  • test run them on JASMIN with the latest dev ESMValCore and obtain results, stock datasets from recipe
  • rerun them with UKESM1 data for comparisons with PP data -> @alistairsellar needs to tell me the data specs
  • store results somewhere easily available for checking
  • run the equivalent Autoassess ones on UKESM pp data
  • compare and do scientific review
  • complete documentation
  • fix CMIP5 (mrlsl) - possibly write a CMIP5-specific recipe or recipe section
  • remove MOHC test recipes
  • add missing plots in gallery (from permafrost only, no other plots outputted)
  • mergy-merge and grab a brewski 🍺

@valeriupredoi
Copy link
Contributor Author

OK - smashing news: I ran all four landsurface recipes with the latest esmval- code from current development branches (both Core and current Tool branch, this one). Specs are:

esmval code

(release220) [valeriu@sci1 esmvaltool_var_test]$ conda list esmval
# packages in environment at /home/users/valeriu/anaconda3/envs/release220:
#
# Name                    Version                   Build  Channel
esmvalcore                2.2.0                     dev_0    <develop>
esmvaltool                2.2.0                     dev_0    <develop>
esmvaltool-sample-data    0.0.3                    pypi_0    pypi

Python, conda, iris and matplotlib

(release220) [valeriu@sci1 esmvaltool_var_test]$ conda list python
# packages in environment at /home/users/valeriu/anaconda3/envs/release220:
#
# Name                    Version                   Build  Channel
antlr-python-runtime      4.7.2           py39hde42818_1002    conda-forge
msgpack-python            1.0.2            py39h1a9c180_1    conda-forge
python                    3.9.2           hffdb5ce_0_cpython    conda-forge
python-dateutil           2.8.1                      py_0    conda-forge
python-stratify           0.1.1           py39hce5d2b2_1003    conda-forge
python-xxhash             2.0.0            py39h3811e60_1    conda-forge
python_abi                3.9                      1_cp39    conda-forge

(release220) [valeriu@sci1 esmvaltool_var_test]$ conda -V
conda 4.10.1

(release220) [valeriu@sci1 esmvaltool_var_test]$ conda list iris
# packages in environment at /home/users/valeriu/anaconda3/envs/release220:
#
# Name                    Version                   Build  Channel
iris                      3.0.1            py39hf3d152e_1    conda-forge
(release220) [valeriu@sci1 esmvaltool_var_test]$ conda list matplotlib
# packages in environment at /home/users/valeriu/anaconda3/envs/release220:
#
# Name                    Version                   Build  Channel
matplotlib                3.3.4            py39hf3d152e_0    conda-forge
matplotlib-base           3.3.4            py39h2fa2bec_0    conda-forge

@valeriupredoi
Copy link
Contributor Author

@alistairsellar could you please give me the UKESM1 data specifications that I should be rerunning these recipes with? If you look at the autoassess_landsurface_* recipes in the current recipes dir, none of the recipes use UKESM1, so if you could tell me what the exact datasets per recipe should be so we can do the comparisons with the PP data, that's be grand! 🍺

@alistairsellar
Copy link
Contributor

alistairsellar commented May 13, 2021

Hi V, please use these runs to allow direct comparison:

  • HadGEM3-GC31-LL r1i1p1f3
  • UKESM1-0-LL r5i1p1f3

The period you have already is fine: start_year: 1992, end_year: 2002
but can I just check that this means 1992-01-01 to 2002-12-31?
We should use a longer period in the recipes that get merged once we're happy, but this shorter period is good for initial testing.

@valeriupredoi
Copy link
Contributor Author

excellent, cheers @alistairsellar 🍺 are the other data parameters and variables good?

@alistairsellar
Copy link
Contributor

Yes, they look correct to me.

@valeriupredoi
Copy link
Contributor Author

excellent @alistairsellar - I'll run those tomorrow after 10am (JASMIN is having FS updates 830am-10am tomorrow, I can have coffee β˜• ) 🍺

@alistairsellar
Copy link
Contributor

Hi again. I've now been through the original code in more detail, and picked up 3 changes to variable names needed in the recipes (see comments above).

I also noticed your comment about the missing soil moisture variables. I'm consulting with our local expert on this.

@valeriupredoi
Copy link
Contributor Author

valeriupredoi commented May 14, 2021

Data issues: CMIP6 data is patchy on JASMIN: so far I have noticed mrsos data missing for HadGEM3-GC31-LL for all experiments, and other bits and bobs missing for the setup you asked for @alistairsellar - for permafrost I could get data (apart for mrsos if I change to historical or piControl, would either of those do?

@alistairsellar
Copy link
Contributor

Any experiment with HadGEM3 or UKESM1 will be fine - I can make the equivalent AutoAssess in a couple of hours. You could even compare two UKESM exps, or two HadGEM3 exps if that's more complete.

@valeriupredoi
Copy link
Contributor Author

valeriupredoi commented May 14, 2021

@alistairsellar can we not do it for the exact same model and experiment - comparing the individual metrics should probably do the trick? At least for permafrost mrsos is giving me terrible headaches for HadGEM missing everywhere. If I do two different experiments for UKESM1 that's no good either because the fx files (sftlf) lives only in piControl (this is a massive gripe me and @zklaus and @ledm have been having, replication of fx files across experiments is nonexistent in CMIP6 UKESM1 data)

@alistairsellar
Copy link
Contributor

alistairsellar commented May 14, 2021

Sure, that's fine. Whatever you can make work for available CMIP6 data, I can replicate here. (So long as it uses either UKESM or HadGEM3 of course!)

@alistairsellar
Copy link
Contributor

alistairsellar commented May 14, 2021

this is a massive gripe...

Is fx thing this a Met Office-specific issue? If so, do you want me to pick it up with our data publishing experts?

@valeriupredoi
Copy link
Contributor Author

valeriupredoi commented Jun 16, 2021

my guess is that your ESGF node has ES3M-1-0 sftlf as a fraction (range 0-1) while the Jasmin/CEDA node has it (correctly) as a percentage. Is it possible that ES3M initially delivered sftlf with the wrong units and subsequently corrected it, and that Jasmin has a newer version of that data than the ESGF node you are pointing to?

fairly sure it's the sftlf data that is two factors lower on DKRZ

[Edit (by Alistair): correcting the quote since I corrected my original post that it was quoting. Also, I think you meant lower on DKRZ, rather than higher. I don't know if it's bad form to edit someone else's post, but I didn't want to confuse Remi...]

Also added original authors
@remi-kazeroni
Copy link
Contributor

So why is E3SM-1-0 wrong in your test, just for these two metrics? Your hunch has got to be right that there is a fraction / percent mixup here. These are the only metrics that make use of the sftlf land area mask, so my guess is that your ESGF node has E3SM-1-0 sftlf as a fraction (range 0-1) while the Jasmin/CEDA node has it (correctly) as a percentage. Is it possible that E3SM initially delivered sftlf with the wrong units and subsequently corrected it, and that Jasmin has a newer version of that data than the ESGF node you are pointing to?

[Edit - I got the models the wrong way round initially. E3SM is the one that is too low by 100.]

Thanks for your investigations @alistairsellar! I'm running the recipes at DKRZ which has an older version of the data you are referring to: /mnt/lustre02/work/ik1017/CMIP6/data/CMIP6/CMIP/E3SM-Project/E3SM-1-0/historical/r1i1p1f1/fx/sftlf/gr/v20190919/sftlf_fx_E3SM-1-0_historical_r1i1p1f1_gr.nc. That file contains indeed only [0-1] values... But I see that the version v20201015 is made available on Jasmin but not on Mistral. That must explain the discrepancies. Sorry for the inconvenience!

@alistairsellar
Copy link
Contributor

That must explain the discrepancies.

Phew! That's good news.

@alistairsellar
Copy link
Contributor

So apart from the small matter that my last commit has broken the tests, I think we're done. Provenance now added, and metrics issue resolved. Anything I've missed?

I'll fix that final error now...

@alistairsellar
Copy link
Contributor

Test now fixed. Back to you @remi-kazeroni - I think we've covered everything raised by your review.

@valeriupredoi
Copy link
Contributor Author

[Edit (by Alistair): correcting the quote since I corrected my original post that it was quoting. Also, I think you meant lower on DKRZ, rather than higher. I don't know if it's bad form to edit someone else's post, but I didn't want to confuse Remi...]

good point and call, Alistair! I am working in 30C at home, I don't even know how me brain is not mush πŸ₯΅

Copy link
Contributor

@remi-kazeroni remi-kazeroni left a comment

Choose a reason for hiding this comment

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

Thanks for adding the provenance @valeriupredoi & @alistairsellar! For each three recipes, a diagnostic_provenance.yml file is created. But 2 of these recipes crash before the end because of the provenance. There seems to be a last bug for soilmoisture and surfrad... Could you please have a look? I attached the main_log_debug.txt and log.txt for one of these.
log.txt
main_log_debug.txt

@remi-kazeroni
Copy link
Contributor

Forgot to ask: do you want to edit the PR's title or is it fine like this for the merge and the changelog?

@alistairsellar alistairsellar changed the title Reading Autoassess landsurface recipes for scientific review and inclusion in ESMValTool v2.3 Autoassess land surface metrics: permafrost, soil moisture, surface radiation Jun 17, 2021
@valeriupredoi valeriupredoi changed the title Autoassess land surface metrics: permafrost, soil moisture, surface radiation Add functional Autoassess diagnostics: land surface metrics: permafrost, soil moisture, surface radiation Jun 17, 2021
@valeriupredoi
Copy link
Contributor Author

cheers @remi-kazeroni (sorry @alistairsellar I re-edited the title 😁 ) - I have also updated #1931 that should not be closed yet since snow still needs attention but now it contains the correct pointers to the work here

@valeriupredoi
Copy link
Contributor Author

Thanks for adding the provenance @valeriupredoi & @alistairsellar! For each three recipes, a diagnostic_provenance.yml file is created. But 2 of these recipes crash before the end because of the provenance. There seems to be a last bug for soilmoisture and surfrad... Could you please have a look? I attached the main_log_debug.txt and log.txt for one of these.
log.txt
main_log_debug.txt

eek! I'll have a look now, cheers mate 🍺

@valeriupredoi
Copy link
Contributor Author

OK @remi-kazeroni - fixed it, silly code was trying to re-write the record when running the bit to plot the metrics, my bad, I should have tested by running the thing yesterday (I have now 😁 )

Copy link
Contributor

@remi-kazeroni remi-kazeroni left a comment

Choose a reason for hiding this comment

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

Everything works fine, thanks a lot @alistairsellar and @valeriupredoi! I can run the three recipes successfully and the provenance is well written in each case. This is good to be merged in my opinion 🍺

@valeriupredoi
Copy link
Contributor Author

brilliant! cheers ever so much, Remi! since you helped so much with it, how about you merge it πŸ…

@remi-kazeroni remi-kazeroni merged commit 34e7f23 into main Jun 17, 2021
@remi-kazeroni remi-kazeroni deleted the autoassess_landsurface_recipes branch June 17, 2021 12:27
@remi-kazeroni
Copy link
Contributor

brilliant! cheers ever so much, Remi! since you helped so much with it, how about you merge it πŸ…

You're welcome @valeriupredoi! Thanks to you and @alistairsellar for all the work! Great to have this merged for the next release and the IS-ENES3 deliverable D9.2 πŸ‘

@alistairsellar
Copy link
Contributor

That's great! Thank you @valeriupredoi & @remi-kazeroni for your time on this! Great to be another step closer to using ESMValTool in operations at the UK Met Office.

@ehogan ehogan added the AutoAssess Issues relevant to the conversion of AutoAssess metrics from Met Office to ESMValTool label Dec 10, 2021
@ehogan ehogan added this to In progress in AutoAssess via automation Dec 10, 2021
@ehogan ehogan moved this from In progress to Done in AutoAssess Dec 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Autoassess diagnostics port (mother of all autoassess issues)
7 participants