Conversation
jonasbhend
left a comment
There was a problem hiding this comment.
Hi @lclanzi. Thanks for implementing this. I think the 'cross-validation' with holdin/out stations is a very nice feature. Given the additional way to select stations from jretrieve, I was wondering if we really want to support bbox selection and filtering down to Switzerland? What is the use case here that is not covered by selection by measurement network (e.g. 1,2) or bbox? If we don't have a strong argument, I would rather avoid this additional complexity.
Also, I am unable to check the implementation on linard, as mlflow is still being blocked. Will try to do a short test on Wednesday to confirm everything is working as expected.
Hi @jonasbhend, thanks a lot for the feedback! |
|
I know that this doesn't work (easily), but my question is: why do you want to use only stations in Switzerland? I guess for the nudging, you wouldn't want to constrain, and for the evaluation we could argue similarly. If anything, I think for the evaluation we should implement it as a stratifiable region (in essence the hull of all forecasting regions), which would then be available for both station and analysis-based evaluations. |
|
Thanks for the suggestions @jonasbhend! |
dnerini
left a comment
There was a problem hiding this comment.
very cool feature! I left some comments
There was a problem hiding this comment.
this config can't be used by anyone else since in multiple instances it points to your personal folders, correct? Can we fix this? Otherwise remove the yaml
There was a problem hiding this comment.
I removed the yaml. I will make a separate PR with varda-rapid related configs
There was a problem hiding this comment.
is adding an experiment yaml really necessary for this new feature PR?
There was a problem hiding this comment.
I removed the yaml. I will make a separate PR with varda-rapid related configs
| return xr.Dataset(data_vars=data_vars, coords=coords) | ||
|
|
||
|
|
||
| def _trim_stations_xr( |
There was a problem hiding this comment.
why is this necessary? can't be handled directly through the jretrieve query?
There was a problem hiding this comment.
In principle, yes. However, I have not found a way to query SwissMetNet + partner stations within Switzerland using jretrieve. What I currently do is to query stations over a rectangular domain containing Switzerland with use_limitation=40, and further trim all stations located out of the Swiss borders. @clairemerker and @jonasbhend also looked into this, and If I remember correctly, you also had issues with this operation?
There was a problem hiding this comment.
As mentioned previously, I don't see the point in doing the subsetting on the jretrieve part. For evalml it seems sufficient to use bbox selection in jretrieve together with region stratification (which now includes Switzerland only as a separate region, right)? So I still suggest to drop this to simplify and keep the loading of obs clean(ish).
There was a problem hiding this comment.
As far as I know, bbox selection only works with a rectangular domain, meaning that stations out of Switzerland will be included in the ground truth. Or am I wrong?
There was a problem hiding this comment.
No that is correct. bbox selection is what it is. But if you wanted stations only from Switzerland, there is probably ways to do that by selecting measurement networks. And in any case for verification you could just use bbox selection and then stratify by Switzerland as a region which should be equivalent to bbox selection and trimming to Switzerland on the jretrieve call. My question here is if there is a use case (for nudging I guess) where we would want to use only stations from Switzerland and not the surrounding areas (other than selecting SwissMetNet say).
There was a problem hiding this comment.
Yes, selecting Switzerland as a region would indeed work. Good point. I will remove this filter.
It would be nice though to have a better understanding of the jretrieve API. I would not know for instance how to select measurement networks
There was a problem hiding this comment.
yep, in evalml, jretrieve:1,2 for example selects the measurement network 1: SMN and 2: SMN Niederschlag
There was a problem hiding this comment.
I think stn_group_id doesn't provide enough options to retrieve the partner stations in Switzerland, at least I didn't manage to do so (either not implemented or not documented?). Maybe using meas_group_id could work, but the list is overwhelming to me, we will need help from MD to understand what to retrieve...
(You can have a look at the options further down on that page: https://service.meteoswiss.ch/jretrieve/api/v1/surface/help)
|
|
||
| # Per-variable station-coverage log, so ground-truth coverage can be compared | ||
| # against what was actually available to nudge. | ||
| _logged_params = ("T_2M", "TD_2M", "U_10M", "V_10M", "PMSL", "TOT_PREC", "VMAX_10M") |
There was a problem hiding this comment.
why hardcoding these variables here?
| command += config.profile.parsable() | ||
| command += ["--configfile", str(configfile)] | ||
| command += ["--cores", str(cores)] | ||
| command += ["--rerun-incomplete"] |
| the config contains a ``forward_transform_filter: | ||
| nudge_toward_observation`` block, in which case its | ||
| ``exclude_stations``/``holdout_fraction``/``holdout_seed`` are | ||
| injected into that block — see ``_inject_nudging_station_holdout``. |
There was a problem hiding this comment.
I think I need to be convinced about this approach as it seems to me a bit too fragile...
Wouldn't be more robust to use the station_holdout_cfg to write a list of stations that is then used downstream, eg for nudging? I know the nudging filter expects a list of stations in its config, but maybe we could change that a path to a list of stations?
This PR adds a new stratification axis, station group (
all/holdout/holdin), alongside the existing region/season/init_hour stratifications, plus supporting infrastructure to make ground-truth retrieval more flexible. Note that this is a single fixed holdout/holdin split used for stratified evaluation, not k-fold cross-validation.all/holdout/holdinstations, so skill can be evaluated separately on stations that were withheld from nudging (or other applications) vs. those that fed into it.experiment.cross_validationin the top-level config (exclude_stations, orholdout_fraction+holdout_seedfor a random split) is the single source of truth for which stations define the holdout groupverify()(src/verification/__init__.py) adds astation_groupdimension to verification outputs, alongside the existing region/season/init_hour dimensions.report_experiment_dashboard.py,template.html.jinja2,script.js) and scorecards (report_scorecard.py) are updated to expose and correctly aggregate this new dimension.verif_hash()incommon.smknow includes the station-group configuration, so changing it correctly invalidates cached verification outputs instead of silently reusing stale results.data_input.jretrieve.parse_selection()now supportsfilter_mode=switzerland(trim to stations within the real Swiss national border) andfilter_mode=domain;domain_bbox=...(trim to an arbitrary bounding box), plus an optionaluse_limitation=<N>selector for data-usage-rights filtering. This lets a truth-root retrieve over a broad domain and then restrict to a precise region before verificationThis PR depends on MeteoSwiss/anemoi-plugins-meteoswiss#40 and should be merged only afterwards.