Add ClimWIP recipe to reproduce Brunner et al. 2019

[lukasbrunner]

Description

This PR adds an additional recipe to the ClimWIP diagnostic with reproduces (part of) the results from an earlier study (Brunner et al. 2019) as proposed in #2105. There are still some open design questions that need resolving, I will discuss those below.

• Closes Add recipe to ClimWIP reproducing Brunner et al. (2019) #2105
• Link to documentation:

---

Before you get started

• ☝ Create an issue to discuss what you are going to do

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.

• 🛠 This pull request has a descriptive title
• 🛠 Code is written according to the code quality guidelines
• 🛠 Documentation is available
• 🛠 Tests run successfully
• 🛠 The list of authors is up to date
• 🛠 Any changed dependencies have been added or removed correctly
• 🛠 All checks below this pull request were successful

New or updated recipe/diagnostic

• 🧪 Recipe runs successfully
• 🧪 Recipe is well documented
• 🧪 Figure(s) and data look as expected from literature
• 🛠 Provenance information has been added

---

[Peter9192]

I reckon you're still working on this? If not, you can mark it ready for review.

[lukasbrunner]

Additional discussion:

Different regions: The original study looks at 8 European sub-regions, I've reduced them to 4 as the other 4 were very experimental and should not be used without careful evaluation.

• Decide how to represent these regions
  • 4 different recipes
  • all in one recipe (requires the pre-processor to run 4 times - slow)
  • all in one recipe but all but one option commented out (current version)
  • only one recipe and documentation how to adapt it

Documentation: The current documentation focuses on explaining the core functionality of ClimWIP and how to write a basic recipe. This needs to be updated to also include this recipe.

• Decide how to structure the documentation
  • If possible: add another level of headings

-ClimWIP
  |-Basic functionality and testing
    |-Overview
    |-... (current level 2 headings)
  |-Brunner et al. (2019)
  |-additional recipes to come... 

• Append the documentation to the existing one
• Restructure the entire documentation and integrate documentation directly

[lukasbrunner]

I reckon you're still working on this? If not, you can mark it ready for review.

Yes and no. There are open questions where I'd be happy for feedback. Can you comment on the code already or do I need to hit the ready for review button?

[Peter9192]

* Decide how to represent these regions

I guess it depends on the diagnostic. If the results for the different regions need to be combined in one plot, then you would need a single recipe. If you don't, I would go for 4 different recipes. Although in that case, I would actually just add one recipe, and document how it can be changed in order to analyze a different region. Does that make sense?

[lukasbrunner]

* Decide how to represent these regions

I guess it depends on the diagnostic. If the results for the different regions need to be combined in one plot, then you would need a single recipe. If you don't, I would go for 4 different recipes. Although in that case, I would actually just add one recipe, and document how it can be changed in order to analyze a different region. Does that make sense?

Yes totally. I would say the idea here is to provide a template how to recreate the weights from the paper and there is no need to create a combined plot. This is basically what I have now I guess. Are you okay with commented lines (representing the other cases) in the recipe? As of now I would leave them in and describe what lines have to be changed to get each of the options from the paper.

[lukasbrunner]

Okay how does this look as template @Peter9192 ? Still needs some cleaning and the corresponding documentation. Will do that once we've settled on a format. I chose the MED SREX region as example to show here as the results are clearest for this region and we show a timeseries plot in the paper that is similar to the one automatically created. (I might update this one a bit to be more similar to the one in the paper)

[Peter9192]

Looks good to me. I left a few small comments.

[ruthlorenz]

I cannot find the shapefile and therefore cannot test this. I think they need to be provided somewhere.

[ruthlorenz]

Works fine with the shapefiles for MED, except the map looks not good, but I guess it needs #2107
Can we provide the shapefiles (at least for MED as this is the example we use here) somehow in ESMValTool? e.g. as in recipes/examples/recipe_extract_shape.yml?

[Peter9192]

Can we provide the shapefiles (at least for MED as this is the example we use here) somehow in ESMValTool?

Maybe @nielsdrost has some idea about that? Otherwise the best I can come up with is to upload them somewhere (maybe as a gist?) and then link to them in the documentation for this recipe.

[lukasbrunner]

About the shape files: I more and more get the feeling that it should be possible to include them in the diagnostic.

I've updated to use region ids

    extract_shape:
    shapefile: shapefiles/srex.shp
    decomposed: True
    method: contains
    crop: true
    ids:
      - 'South Europe/Mediterranean [MED:13]'

so we would only need one file for all SREX regions and the files are quite small

ls -hl srex.*
-rw-r--r-- 1 lukbrunn wheel 4.1K Jun 25  2015 srex.dbf
-rw-r--r-- 1 lukbrunn wheel  132 Jun 25  2015 srex.prj
-rw-r--r-- 1 lukbrunn wheel 4.6K Jun 25  2015 srex.shp
-rw-r--r-- 1 lukbrunn wheel  364 Jun 25  2015 srex.shx

These I've downloaded these from http://www.ipcc-data.org/guidelines/pages/ar5_regions.html
The only disadvantage in using this file directly is that it seems it requires the long name (South Europe/Mediterranean [MED:13]) instead of the more common abbreviations (MED) as ID.

[Peter9192]

PR looks good now, but we should make sure the tests pass before we can ask the core team to merge it.

[Peter9192]

@ESMValGroup/esmvaltool-coreteam can you help us solve the CI issue?

The PR uses a new feature from ESMValGroup/ESMValCore#764, where the ids argument was added to extract_shape. However, the tests complain that this is not a valid argument. Are they using an old version of ESMValCore? Is this something we can solve in this PR?

[valeriupredoi]

the CI tests are running against an old esmvalcore, if you install the latest esmvalcore from the master development branch, tests pass fine. This is happening because in the CI environment esmvalcore is installed from conda as the released package and I don't think the ID's PR went in before the release?

[valeriupredoi]

yeah no, the ID's stuff went in 12 March, we released 2.2.0 February 9th 👍

[Peter9192]

@valeriupredoi thanks, so this is intended, and the solution is just to wait for the next release of the core?

I might have misunderstood that the container that runs the tests is the development version of the core...

[lukasbrunner]

Just to follow up on this (and so that I know what to do with this on my todo list): we have to wait for the next core release? @valeriupredoi

[bouweandela]

Yes, I thought I had now written this down in the developer documentation, but I cannot find it at the moment. The ESMValTool repo uses the latest released version of ESMValCore in order to provide a stable development environment to diagnostics developers. That way we have the freedom to make changes to the ESMValCore quickly, without worrying too much about breaking everything for everyone, as we'll probably catch the problem before making the next ESMValCore release.

[bouweandela]

You might want to add this pull request to the v2.3 milestone, to make sure it is visible.

[lukasbrunner]

ping @Peter9192 @valeriupredoi
I guess this should have worked now? It still fails with the same error though...

[lukasbrunner]

Here is the logfile from my run with 2.3.0
main_log.txt

[bouweandela]

You need to pull in ESMValCore 2.3 as a dependency, that will only work #2200 has been merged into main and you merge main into this branch.

[Peter9192]

@ESMValGroup/esmvaltool-coreteam this PR is ready for merge 👍

[hb326]

@ESMValGroup/esmvaltool-coreteam this PR is ready for merge 👍

Done!