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

Implement shape parameter calibration for ClimWIP #1905

Merged
merged 68 commits into from Feb 17, 2021

Conversation

lukasbrunner
Copy link
Contributor

@lukasbrunner lukasbrunner commented Nov 24, 2020

Tasks

  • Create an issue to discuss what you are going to do, if you haven't done so already (and add the link at the bottom)
  • Give this pull request a descriptive title that can be used as a one line summary in a changelog
  • Make sure your code is composed of functions of no more than 50 lines and uses meaningful names for variables
  • Circle/CI tests pass. Status can be seen below your pull request. If the tests are failing, click the link to find out why.
  • Preferably Codacy code quality checks pass, however a few remaining hard to solve Codacy issues are still acceptable. Status can be seen below your pull request. If there is an error, click the link to find out why. If you suspect Codacy may be wrong, please ask by commenting.
  • Please use yamllint to check that your YAML files do not contain mistakes

Modified recipe/diagnostic

  • Update documentation for the recipe to the doc/sphinx/source/recipes folder
  • Update provenance information if needed
  • Assign the author(s) of the affected recipe(s) as reviewer(s)

If you need help with any of the tasks above, please do not hesitate to ask by commenting in the issue or pull request.


Closes #1902
Documentation: https://esmvaltool--1905.org.readthedocs.build/en/1905/recipes/recipe_climwip.html

@lukasbrunner lukasbrunner added diagnostic EUCP www.eucp-project.eu labels Nov 24, 2020
@lukasbrunner lukasbrunner self-assigned this Nov 24, 2020
@lukasbrunner lukasbrunner changed the title Implement on the fly shape parameter claibration Implement shape parameter claibration for ClimWIP Nov 24, 2020
@lukasbrunner
Copy link
Contributor Author

Hey @Peter9192 ! I now have a first running version. If you have time tomorrow at some point we could discuss it. It is quite similar to my pseudo code idea with the addition of the scipy.minimize

@Peter9192
Copy link
Contributor

Hey @lukasbrunner I'm on it now. I totally see why you made the utilities file, but it's quite hard to compare the changes now. Do you think you could split up the commits a bit more logically (maybe move everything first without changes, then the actual changes)? That would make it much easier to go commit by commit and evaluate the relevant changes only.

@lukasbrunner
Copy link
Contributor Author

lukasbrunner commented Nov 25, 2020

Ah sorry yes! I always get ahead of myself and then there are no atomic commits any more... but really the changes to climwip.py are limited to moving functions to utilities.py and utilities.py contains only functions from the original climwip.py. So the only real changes are isolated in calibrate_sigmas.py (except for the call in climwip.py and a very small piece cross-checking if sigmas are already set)

Do you still want me to redo this? Might be a good learning process for me anyway so that would be okay :)

@Peter9192
Copy link
Contributor

Let's discuss it first

Comment on lines 143 to 146
if confident:
return performance_sigma
return 9999 - performance_sigma

Copy link
Contributor

Choose a reason for hiding this comment

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

Apologies for cross posting 😋

Maybe this is still too simple, but I wanted to offer a thought. If we specify confidence as a spectrum around 0.80, where anything below 0.80 is overconfident (and lower is worse), and anything above 0.80 as 'underconfident' (diffident? ; higher is worse), then minimizing something like abs(confidence - target_confidence) where target_confidence=0.80 could maybe work?

Lukas Brunner added 12 commits November 26, 2020 14:26
- calculate_model_distances: update docstring
- calculate_model_distances: add argument to name new dimension name
- compute_overall_mean: simplify median calculation
- combine_ensemble_members: update docstring
- combine_ensemble_members: allow passing of dimension names to be
combined
- calculate_weights: move actualy calculation to a new function to get
more flexible. This function becomes a wrapper doing xarray stuff
@lukasbrunner
Copy link
Contributor Author

Hey @Peter9192 !
My next try with what I think is the most basic version of the sigma calibration :) I'm aware that its not quite polished now and the code still contains a couple of open questions.

  • I now use brute instead of minimize. I realize that this might be slightly less elegant but it is more practicable because it allows a plot of what is going on. I think this is quite important to evaluate if the perfect model test did work. In addition it makes the exact definition of the cost function less important.
  • Not sure if my method of extracting the values from the parallel loop inside brute using dict is quite how this should be done...
  • I realize the plot is a bit busy and not easy to interpret but I tried to include all the relevant information and I can add a description to the docs
  • I added a new recipe for running the sigma calibration *_test_performance_sigma.yml (the old one did not have enough model for meaningful results) and renamed the old one into *_test_basic.yml. Not sure if this is something that is used in ESMValTool but in the end I think it needs to be clear that both of them are exactly that: toy examples with out much real meaning. Once Ruth is done we can add her recipe as 'real' application.

Example figure sigma calibration

performance_sigma_calibration
Figure: Ratio of perfect models inside of the weighted 80% range (thick black line) depending on the performance sigma value. The smallest not-overconfident sigma is marked by the dashed vertical line. Ratio of weighted/unweighted 80% ranges (min, mean, max; gray line and shading) depending on the performance sigma value. Cost function for finding the ideal sigma value (red, right axis).

Additional information:
I also show the reference we try to reach (dashed horizontal line) and the unweighted baseline (doted horizontal line). The difference between the two is that for the baseline the perfect model is excluded from the distribution meaning baseline <= reference and baseline = reference for number of models -> infinity. In addition the ratio of perfect models -> unweighted baseline for sigma -> infinity

I also show a measure of "sharpness", i.e., the ratio between weighted and unweighted spread for each perfect model. The smaller the value the stronger the constraint of the weighting, with sharpness -> 1 for sigma -> infinity.

The cost function needs to basic properties: If no confident sigma can be found it needs to pick the largest possible value (weakest weighting) and for all confident values it needs to pick the smallest possible value.

@stefsmeets stefsmeets changed the title Implement shape parameter claibration for ClimWIP Implement shape parameter calibration for ClimWIP Dec 3, 2020
@stefsmeets stefsmeets self-requested a review December 3, 2020 09:40
@esmvalbot
Copy link

esmvalbot bot commented Feb 4, 2021

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

@esmvalbot
Copy link

esmvalbot bot commented Feb 4, 2021

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

@Peter9192
Copy link
Contributor

@ruthlorenz would you have time to do the scientific review this week? If you can try if it runs for you on the ETH resources, then I can stop trying with the bot..

@ruthlorenz
Copy link
Contributor

yes I can do a scientific review

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.

I added a few little things that need changing. Otherwise looks good to me and runs fine.
However, I am not completely independent since I run on the same servers as @lukasbrunner.

doc/sphinx/source/recipes/recipe_climwip.rst Outdated Show resolved Hide resolved
doc/sphinx/source/recipes/recipe_climwip.rst Outdated Show resolved Hide resolved
@lukasbrunner
Copy link
Contributor Author

Thanks @ruthlorenz for the careful check!!
@Peter9192 not sure why CI fails now I only touched comments since my earlier commits. (except maybe the added information to the recipe?) Or the merge of the master?

@Peter9192
Copy link
Contributor

Seems unrelated to the PR, something with the test_cmorize_obs. Guessing this might be due to some of the refactoring of the logger we did recently in the core (@stefsmeets ?).

@stefsmeets
Copy link
Contributor

Seems unrelated to the PR, something with the test_cmorize_obs. Guessing this might be due to some of the refactoring of the logger we did recently in the core (@stefsmeets ?).

I don't know what happened with this, should have been fixed by #2020

@valeriupredoi
Copy link
Contributor

#2020 fixed it for the current development master branch of ESMValCore, but broke it for the released version which is used for the CI tests stirring the wrath of @bouweandela while at it 😁

@Peter9192
Copy link
Contributor

One more minor technical remark @lukasbrunner, can you run yamllint on the changed recipes? I think you might want to trim down the line length on some of the descriptions you've added.

@Peter9192
Copy link
Contributor

Peter9192 commented Feb 15, 2021

Thanks!
Since the CI issue has nothing to do with this PR I'm happy to see this get merged

@ESMValGroup/esmvaltool-coreteam

@bouweandela
Copy link
Member

Could you please merge the master branch into this pull request, so all the tests pass?

@valeriupredoi
Copy link
Contributor

I take it that @ruthlorenz ok-ed this scientologically and @Peter9192 technically right? :grin: I am going to change labels and pending test passing I am going to merge, please object if otherwise :+1:

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.

All good now

@valeriupredoi valeriupredoi merged commit 1e5b5dd into master Feb 17, 2021
@valeriupredoi valeriupredoi deleted the climwip_sigma_calibration branch February 17, 2021 17:43
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.

Implement perfect model test for ClimWIP
6 participants