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

Version2 emergent constraints #1011

Merged
merged 39 commits into from
Oct 29, 2019

Conversation

axel-lauer
Copy link
Contributor

This PR adds the following emergent constraints for equilibrium climate sensitivity (ECS):

  • southern ITCZ index, similar to fig. 2 from Tian (2015)
  • lower tropospheric mixing index (LTMI), similar to fig. 5 from Sherwood et al. (2014)
  • tropical mid-tropospheric humidity asymmetry index, similar to fig. 4 from Tian (2015)
  • covariance of shortwave cloud reflection (Brient and Schneider, 2016)
  • climatological Hadley cell extent (Lipat et al., 2017)

The new recipes (recipe_ecs_scatter.yml and recipe_ecs_scatter_cmip6.yml) have been tested with CMIP5 and CMIP6 data. The code works fine but CirecleCI reports the following error for the newly added CMIP6 fix files:

collection failure
esmvaltool/cmor/_fixes/CMIP6/BCC_CSM2_MR.py:6: in
from ..fix import Fix
E ImportError: attempted relative import with no known parent package

The files in question are:

  • esmvaltool/cmor/_fixes/CMIP6/BCC_CSM2_MR.py
  • esmvaltool/cmor/_fixes/CMIP6/BCC_ESM1.py

The reason for the complaints by CircleCI are not clear to me. Advice welcome!

@axel-lauer
Copy link
Contributor Author

PR #1012 seems to fix the problems the CircleCI complaints about the CMIP6 fixes.

@schlunma
Copy link
Contributor

schlunma commented Apr 8, 2019

Yes, it was the missing __init__.py file. A

git cherry-pick 510dabaabd20064350018a9b00f8559aefdf3b98

in your branch should fix it 👍

@axel-lauer
Copy link
Contributor Author

Changed figure labels to avoid duplicate labels. Thanks for pointing out this problem. Not sure what you mean by scrambled documentation, though.

@JaroCamphuijsen
Copy link
Contributor

JaroCamphuijsen commented Sep 12, 2019

@axel-lauer the time operations of the preprocessor have recently been refactored and renamed in ESMValGroup/ESMValCore#87 . Unfortunately this means you have to revise the time preprocessing you do in your recipe and/or diagnostic. For more information see the new set of time operations (old functionality still available but named differently and generalized).

@axel-lauer
Copy link
Contributor Author

@JaroCamphuijsen not sure what you mean as no time operations are done by the preprocessor in the recipe added in this PR. Just merged the latest version2_development into this branch and tested the new recipe. Works fine.

@axel-lauer
Copy link
Contributor Author

@JaroCamphuijsen I think it is better to sort by actual headline that will show up in the user's guide than by file name appearing nowhere but in index.rst:

emergent_constraints: Emergent constraints for equilibrium climate sensitivity
ecs: Equilibrium climate sensitivity

So I will not change the current position of "recipe_emergent_constraints" in index.rst.

@JaroCamphuijsen
Copy link
Contributor

@JaroCamphuijsen I think it is better to sort by actual headline that will show up in the user's guide than by file name appearing nowhere but in index.rst:

I agree with you that the current way is not necessarily the best way, but I think this is currently how this file is sorted, so we should stick with it. If you want to suggest to do it your way maybe you can start an issue where we can discuss this and then we can apply it to all recipes in index.rst

That said, maybe letting the merge depend on this discussion is not worth it, the rest look fine to me. @mattiarighi could you review this so it can be merged?

@axel-lauer
Copy link
Contributor Author

@JaroCamphuijsen the convention how files are sorted in index.rst was and is in alphabetical order sorted by headline, not by filename. That is because we do not have any conventions for naming the files, so authors are free to use e.g. acronyms to name their .rst file. As the of the user's guide reader cannot see the file names but only the actual headlines, we sort by headline. This is not up for discussion as the user's guide will become a mess otherwise.

We can, however, discuss introducing a naming scheme for the individual .rst files, but I currently do not see a pressing need for that.

@mattiarighi
Copy link
Contributor

There is an issue with a custom variable (prStderr) in obs4mips:

2019-09-19 10:32:10,888 UTC [30955] INFO    Creating preprocessor task itczidx/prStderr
2019-09-19 10:32:10,888 UTC [30955] INFO    Creating preprocessor 'default' task for variable 'prStderr'
2019-09-19 10:32:11,237 UTC [30955] ERROR   Program terminated abnormally, see stack trace below for more information
Traceback (most recent call last):
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/_main.py", line 228, in run
    conf = main(args)
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/_main.py", line 156, in main
    process_recipe(recipe_file=recipe, config_user=cfg)
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/_main.py", line 202, in process_recipe
    recipe = read_recipe_file(recipe_file, config_user)
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/_recipe.py", line 71, in read_recipe_file
    recipe_file=filename)
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/_recipe.py", line 838, in __init__
    self.tasks = self.initialize_tasks() if initialize_tasks else None
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/_recipe.py", line 1170, in initialize_tasks
    task_name=task_name,
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/_recipe.py", line 783, in _get_preprocessor_task
    _add_cmor_info(variable)
  File "/mnt/lustre01/pf/b/b309057/ESMValTool/core/esmvalcore/_recipe.py", line 122, in _add_cmor_info
    format(cmor_table, short_name, mip))
esmvalcore._recipe_checks.RecipeError: Unable to load CMOR table 'obs4mips' for variable 'prStderr' with mip 'Amon'
2019-09-19 10:32:11,255 UTC [30955] INFO    If you suspect this is a bug or need help, please open an issue on https://github.com/ESMValGroup/ESMValTool/issues and attach the run/recipe_*.yml and run/main_log_debug.txt files from the output directory.

I'm afraid this is due to the recent update to the cmor tables in core.
@jvegasbsc can you have a look?

@jvegreg
Copy link
Contributor

jvegreg commented Sep 20, 2019

Ok. Problem found

  • prStderr is not defined in obs4mips tables
  • We have updated the config developer to make obs4mips in strict mode (not looking at the defaults except when it is a derived variable):
obs4mips:
  cmor_strict: true
  input_dir:
    default: 'Tier{tier}/{dataset}'
    RCAST: '/'
  input_file: '{short_name}_{dataset}_{level}_{version}_*.nc'
  input_fx_dir:
    default: 'Tier{tier}/{dataset}'
  input_fx_file:
    default: '{project}_{dataset}_fx_{fx_var}.nc'
  output_file: '{project}_{dataset}_{level}_{version}_{short_name}_{start_year}-{end_year}'
  cmor_type: 'CMIP6'
  cmor_path: 'obs4mips'
  cmor_default_table_prefix: 'obs4MIPs_'

If we can have obs4mips-like variables that are not part of its tables, we should revert the change and set cmor_strict to False

@axel-lauer
Copy link
Contributor Author

@jvegasbsc Please revert the change for obs4mips from "cmor_strict: true" back to "cmor_strict: false" in config-developer.yml. As the obs4mips tables do not contain (their own) error variables, we had to add custom cmor tables for those variables. Being able to process error variables from obs4mips (and other observational products!) is absolutely essential for any advanced model evaluation.

@axel-lauer axel-lauer merged commit a79a68d into version2_development Oct 29, 2019
@axel-lauer axel-lauer deleted the version2_emergent_constraints branch October 29, 2019 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants