Conversation
Louis-Frey
force-pushed
the
MRB-650-Maps-simplified
branch
from
2185fd6 to
9eb4643
Compare
…oard" This reverts commit cdefa16.
summary statistics. (No changes to code yet.)
of summary statistics.
For Bias, RMSE and MAE map plots.
earthkit instead.
Francesco. Got a long way towards the png plots. Co-authored-by: Francesco Zanetta <francesco.zanetta@meteoswiss.ch>
properly working). Output written to .png now working.
detailed inspection of results at smaller spatial scale.
symmetric colour map for bias.
to see if all of it still works.
By doing this, verification will have to be re-computed for every run, every time the workflow is executed. I don't think we want this, no? |
Yes, that could be problematic. Jonas introduced it and I haven't given it much thought, should I change it back? |
|
Ok I most likely fixed problem 2. See commit 7b50809 |
|
And I just tried again an |
This does the same thing with less code.
| # The domains which are originally called "centraleurope" and "switzerland" | ||
| # are mostly the same. I suggest making domain "switzerland" much smaller, | ||
| # so that more spatial detail can be seen, especially in the complex | ||
| # topography of the alps. |
There was a problem hiding this comment.
it's a good idea! please move tihs comment to the description of your PR where you summarize the main changes introudced by this PR
There was a problem hiding this comment.
The description is the top post in this thread I guess? I put it there too.
|
|
||
| # Load, de-duplicate lead_time, and keep best provider per source (same logic as verif_plot_metrics) | ||
| dfs = [xr.open_dataset(f) for f in args.verif_files] | ||
| drop_variables = ["TOT_PREC.MAE.spatial", "TOT_PREC.RMSE.spatial", "TOT_PREC.BIAS.spatial", |
There was a problem hiding this comment.
instead of hardcoding the variables names, why not just opening the first dataset in the verif_files list and extract all variable names including the keyword "spatial"?
| ds, | ||
| forbidden_dims=("values",), # critical! | ||
| metric_dims=("source", "season", "init_hour", "region", "lead_time", "eps"), | ||
| ) |
There was a problem hiding this comment.
what's the purpose of this change? the spatial fields should have no impact on this part of the code, or am I missing something?
There was a problem hiding this comment.
Yes, the spatial fields don't matter here, but they caused memory problems in the past when this rule was executed. I therefore implemented this work-around so that the spatial parts of the verification files are not loaded into memory in the first place. Chat-GPT assisted in the creation of this code, so forgive me if something is off.
There was a problem hiding this comment.
OK I see, but I wonder if you need it? I mean, at that point you don't expect any spatial field to be present, since those were all filtered out with drop_variables no?
There was a problem hiding this comment.
Ah, yes I tried that. But using drop_variables alone did not solve the problem, the spatial data were still loaded into memory. So I had to add the rest of this code.
There was a problem hiding this comment.
then it'd be good to understand why that isn't behaving as expected and use only one single strategy to get rid of the spatial fields
and similar style for MAE and RMSE of T2m.
Now works!
Some SLURM optimizations.
Change back (clean up in the end)
Might have to slightly adjust for ICON (emulator).
Solves a Problem that occurred due to the merge.
frazane
left a comment
frazane
left a comment
There was a problem hiding this comment.
Beware you added some log files and some notebooks, I don't know if this was intended!
Thanks for pointing out. No, it was not. I will do a cleanup commit at the end where this will be removed. |
4 participants
Add maps of forecast verification scores
Changes
verif.ncverif.nctemporary to avoid storage of large data volumes