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

Fix bug in ClimWIP brunner19 recipe when plotting #2226

Merged
merged 4 commits into from
Sep 10, 2021

Conversation

lukasbrunner
Copy link
Contributor

Description

Rename two parameter keys in the recipe to fix a bug in plotting as described in #2224.


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


To help with the number of pull requests:

@lukasbrunner lukasbrunner added bug EUCP www.eucp-project.eu labels Jul 7, 2021
@lukasbrunner lukasbrunner added this to the [locked] v2.3.0 milestone Jul 7, 2021
@lukasbrunner lukasbrunner self-assigned this Jul 7, 2021
@lukasbrunner
Copy link
Contributor Author

Hoi @zklaus @bouweandela ! My go-to reviewer @ruthlorenz is currently on vacation so I hope you can have a quick look at this. It should fix the problems that @bouweandela found in #2198 (minus the missing models...)

@bouweandela
Copy link
Member

@esmvalbot Please run recipe_climwip_brunner2019_med.yml

@esmvalbot
Copy link

esmvalbot bot commented Jul 8, 2021

Since @bouweandela asked, ESMValBot will run recipe recipe_climwip_brunner2019_med.yml as soon as possible, output will be generated here

@@ -276,13 +276,13 @@ diagnostics:

Copy link
Contributor

Choose a reason for hiding this comment

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

For the other PRs, you added some settings here. Are they not needed for in this case?

@esmvalbot
Copy link

esmvalbot bot commented Jul 11, 2021

ESMValBot is sorry to report it failed to run recipe recipe_climwip_brunner2019_med.yml: exit is null, output has been generated here

@bouweandela
Copy link
Member

@esmvalbot Please run recipe_climwip_brunner2019_med.yml

@esmvalbot
Copy link

esmvalbot bot commented Jul 11, 2021

Since @bouweandela asked, ESMValBot will run recipe recipe_climwip_brunner2019_med.yml as soon as possible, output will be generated here

@esmvalbot
Copy link

esmvalbot bot commented Jul 11, 2021

ESMValBot is sorry to report it failed to run recipe recipe_climwip_brunner2019_med.yml: exit is 1, output has been generated here

@Peter9192
Copy link
Contributor

The bot failed because of missing data and a missing shapefile.

@bouweandela do you think it is possible to make the bot find some shape files? This one seems quite relevant for climate, so perhaps it could even serve as a 'standard' shape file for testing recipes.

@bouweandela
Copy link
Member

@esmvalbot Please run recipe_climwip_brunner2019_med.yml

@esmvalbot
Copy link

esmvalbot bot commented Jul 12, 2021

Since @bouweandela asked, ESMValBot will run recipe recipe_climwip_brunner2019_med.yml as soon as possible, output will be generated here

@esmvalbot
Copy link

esmvalbot bot commented Jul 12, 2021

ESMValBot is sorry to report it failed to run recipe recipe_climwip_brunner2019_med.yml: exit is 1, output has been generated here

@bouweandela
Copy link
Member

I fixed the configuration of the bot so it finds the shapefile, but unfortunately a lot of the required climate model data is not available either on Mistral.

@lukasbrunner
Copy link
Contributor Author

For this recipe (unlike for the testing cases addressed in #2225 & #2227) I can not really change the models as we are trying to re-create a published study with it. So what is the suggested solution here?

@bouweandela
Copy link
Member

I think that the best solution would be if we ask for the additional data to be added: https://www.dkrz.de/up/services/data-management/cmip-data-pool. I'm on holiday this week, but i can look into that next week, or maybe @Peter9192 could do it?

@Peter9192
Copy link
Contributor

I've put in a request for the following datasets:

- {short_name: [tas, pr, rsds, rlds, rsus], dataset: MRI-ESM1, project: CMIP5, exp: rcp85, ensemble: r1i1p1}
- {short_name: [tas, rsds], dataset: GISS-E2-R, project: CMIP5, exp: rcp85, ensemble: r2i1p1}
- {short_name: [pr, rsds], dataset: GISS-E2-R, project: CMIP5, exp: rcp85, ensemble: r2i1p3}
- {short_name: rlds, dataset: CCSM4, project: CMIP5, exp: rcp85, ensemble: r1i1p1}

hope I didn't miss one.

@zklaus
Copy link
Contributor

zklaus commented Jul 13, 2021

Great, thanks, @Peter9192 !

@bouweandela
Copy link
Member

@Peter9192 Did you ever get a response?

@Peter9192
Copy link
Contributor

No, haven't heard anything..

@zklaus
Copy link
Contributor

zklaus commented Sep 1, 2021

Since mistral is not responsive, shall we rely on local running as a means of confirmation? In that case, what is still open?

@bouweandela
Copy link
Member

bouweandela commented Sep 2, 2021

In that case, what is still open?

A review, maybe @ruthlorenz is back from holiday and could have a look?

@ruthlorenz
Copy link
Contributor

ruthlorenz commented Sep 2, 2021

Yes I can have a look! Will do later today or tomorrow....
Regarding the missing data this might be something to be discussed together with the testing strategy?

@bouweandela
Copy link
Member

Hopefully the missing data problem will disappear once ESMValGroup/ESMValCore#1217 has been merged and is available in v2.4 of the ESMValCore.

@zklaus zklaus modified the milestones: [locked] v2.3.0, v2.4.0 Sep 2, 2021
@ruthlorenz
Copy link
Contributor

ruthlorenz commented Sep 3, 2021

I tried to run with ESMValTool v2.3
for some reason it crashes in weighted_temperature_graph (which seems to be unrelated to the changes in here....)
main_log.txt
log.txt

the map works and looks fine.
@lukasbrunner is it up to date with main or is it possible that given that this took so long (sorry.....) that it needs to be updated with main?

@lukasbrunner
Copy link
Contributor Author

Thanks for the test @ruthlorenz! I've merged main into the branch and fixed the bug in the plotting script that created. It runs for me now with everything up to date. Can you try again please?

@ruthlorenz ruthlorenz self-requested a review September 8, 2021 07:28
Copy link
Contributor

@ruthlorenz ruthlorenz left a comment

Choose a reason for hiding this comment

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

Looks good to me and works perfectly fine now!

@bouweandela
Copy link
Member

Has the documentation of the recipe been updated? I didn't see the new settings of the diagnostic script mentioned here: https://docs.esmvaltool.org/en/latest/recipes/recipe_climwip.html

@lukasbrunner
Copy link
Contributor Author

Hi @bouweandela could you specify which settings you are missing? The documentation should be under 'User settings in recipe' generally and under 'Updating the Brunner et al. (2019) recipe for new regions' specifically for this recipe.

@bouweandela
Copy link
Member

bouweandela commented Sep 10, 2021

The settings that are added to the recipe in this pull request: according to the documentation, diagnostic script weighted_temperature_graph.py only takes the arguments ancestors and weights, yet in this pull request an additional settings map containing several keys (central_estimate, lower_bound, start_year, etc) is given to this diagnostic script.

@bouweandela bouweandela merged commit f9350ab into main Sep 10, 2021
@bouweandela bouweandela deleted the fix_bug_climwip_brunner19 branch September 10, 2021 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug EUCP www.eucp-project.eu
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug in ploting routines for ClimWIP test recipes
5 participants