Refactor leaderboard code #990
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ph-kev
force-pushed
the
kp/leaderboard
branch
20 times, most recently
from
7e866ff to
441d700
Compare
Sbozzolo
reviewed
Oct 6, 2024
ph-kev
force-pushed
the
kp/leaderboard
branch
3 times, most recently
from
1b8a197 to
9030e9d
Compare
I moved all the code involving data and preprocessing to |
Sbozzolo
reviewed
Oct 7, 2024
| seasons = ["ANN", "MAM", "JJA", "SON", "DJF"] | ||
|
|
||
| # Print dates for debugging | ||
| pr_var = sim_var_dict["pr"]() # it shouldn't matter what short name we use |
There was a problem hiding this comment.
Instead of assuming that pr is available, you can pick the first entry in the dict
Sbozzolo
reviewed
Oct 7, 2024
Comment on lines
91
to
90
| if season != "ANN" | ||
| CairoMakie.save( | ||
| joinpath(leaderboard_base_path, "bias_$(first(compare_vars_biases))_$season.png"), | ||
| fig_bias, | ||
| ) | ||
| else | ||
| CairoMakie.save( | ||
| joinpath(leaderboard_base_path, "bias_$(first(compare_vars_biases))_total.png"), | ||
| fig_bias, | ||
| ) | ||
| end |
There was a problem hiding this comment.
Suggested change
| if season != "ANN" | |
| CairoMakie.save( | |
| joinpath(leaderboard_base_path, "bias_$(first(compare_vars_biases))_$season.png"), | |
| fig_bias, | |
| ) | |
| else | |
| CairoMakie.save( | |
| joinpath(leaderboard_base_path, "bias_$(first(compare_vars_biases))_total.png"), | |
| fig_bias, | |
| ) | |
| end | |
| CairoMakie.save( | |
| joinpath(leaderboard_base_path, "bias_$(first(compare_vars_biases))_$season.png"), | |
| fig_bias, | |
| ) |
It's fine to change the name to _ANN
Sbozzolo
reviewed
Oct 7, 2024
ph-kev
force-pushed
the
kp/leaderboard
branch
2 times, most recently
from
57eb779 to
4e03a02
Compare
Sbozzolo
approved these changes
Oct 9, 2024
ph-kev
force-pushed
the
kp/leaderboard
branch
from
aac5f66 to
7479d4d
Compare
juliasloan25
approved these changes
Oct 10, 2024
I don't think this is rebased on the latest main. I am going to rebase and push right now. |
ph-kev
force-pushed
the
kp/leaderboard
branch
2 times, most recently
from
0fde103 to
458cb92
Compare
This commit refactors the leaderboard code using the new features from ClimaAnalysis. This commit also deletes the old leaderboard code and the tests for it. Also, there is an off by one month issue when handling the dates and seasons. This arises because the simulation data assigns the next month the monthly average for this month. For instance, the monthly average for January 2010 is assigned the date 2/1/2010. This is fixed in this commit. The commit also moves the code to leaderboard.jl and data_sources.jl. The file read_amip.jl calls leaderboard.jl to plot now. The conditional to run leaderboard.jl is changed from t_end > 86400 to t_end > 86400 * 31 * 3 since it doesn't make sense to run the code unless monthly averages are computed. This is because the dataset contains monthly averages. One significant difference is the leaderboard which now plot the best and worst single model using only annual rather than averaging the error over annual and seasonal data.
ph-kev
force-pushed
the
kp/leaderboard
branch
from
458cb92 to
7777e54
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #948 , closes #956
This PR refactors the leaderboard code using the new features from ClimaAnalysis. This commit also deletes the old leaderboard code and the tests for it. Also, there is an off by one month issue when handling the dates and seasons. This issue arises because the first day of each month in the simulation data represents the monthly average of the previous month. This issue only affect the plots involving seasonal data. This is fixed in this commit.
The commit also moves the code to leaderboard.jl and a line is added to the pipeline to run the script.
One significant difference is the leaderboard which now plot the best and worst single model using only annual rather than averaging the error over the annual and seasonal data.
The leaderboard now looks like this:
To compare, the previous leaderboard looks like this:
The errors for the seasonal data change slightly because of the off by one month issue. The new bias plot for MAM looks like this:
The old bias plot for MAM looks this:
In addition to the plots above, there is now a plot with all the seasons for a subset of the variables (see
compare_vars_biases_groupsfor which variables are plotted together in data_sources.jl).