Adapt probtest workflow to LETKF outputs #83
Conversation
There was a problem hiding this comment.
Thanks a lot for this PR! :)
I have a broader question regarding the logic we are trying to implement for fof_compare, please have a look at https://github.com/MeteoSwiss/probtest/pull/83/changes#r2676392953 before looking at my other comments. Maybe we can discuss, I am not sure I am remembering what we decided correctly, and I might be confused here...
| (ds_obs1_sorted, ds_obs2_sorted), | ||
| ]: | ||
| t, e = compare_var_and_attr_ds(ds1, ds2, nl, output, location) | ||
| t, e = compare_var_and_attr_ds(ds1, ds2, nl, output, location, tol) |
There was a problem hiding this comment.
I have a question here. What is the difference between this code block and what happens in check_multiple_solutions_from_dict(). Can't we reuse this function here?
The main difference with the current PR is that tol=0 in check_multiple_solutions_from_dict(), and this made me think about the fof_compare implementation: don't we want more or less the same check happening in buildbot than when running fof_compare? So tolerances for veri_data, not the rest? Or am I confused here, this could very well be because we might have discussed multiple options... But I somehow expected the tolerance variable to be used for check_variable and veri_data, so that basically check_multiple_solutions_from_dict() could be uses as it, and then this line used
probtest/util/dataframe_ops.py
Line 379 in 43344d3
Please let me know if that doesn't make any sense in your opinion...
This merge request aims to adapt the protest workflow to LETKF outputs.