Add feature to evaluate experimental models that produce their own GRIB files (e.g. HiRAD, WeatherGenerator) - #253
Add feature to evaluate experimental models that produce their own GRIB files (e.g. HiRAD, WeatherGenerator)#253marymcglo wants to merge 9 commits into
Conversation
… experiment-model type that supplies its own GRIB files, skipping the inference step.
|
This is quite a large change and I was wondering how much of this is necessary given that we already have forecast data for which grib files exist (i.e. baseline). Would it also work to just add hirad as a baseline (no config, data import change) and make sure that baselines and runs are interchangeable (i.e. that we can also produce a scorecard with one baseline against another)? |
I think this hits on the fundamental question of whether we want to support bring-your-own GRIB (or more generally, skipping inference in If we do, then I agree this is not the best implementation. We should instead change the Snakemake workflow to add an option to skip inference, and modify the RunConfig definition to be more generic, and have something like RunFromCheckpoint and RunFromGrib subclasses. If not, then I still believe this approach (subclassing BaseModel infra but allowing it to be defined as an experiment) is the best workaround we have (though I think the passthrough changes on the .smk files can be cleaned up quite a bit). In my opinion, requiring such use cases to be defined as baselines, and then adding the ability to compare baselines, would be an even larger workaround that fundamentally changes the API and could lead to further unintended use down the line. (Hyrum's law) I'm open to discussing further, especially if supporting bypassing inference is something we could incorporate into the refactor. |
|
Added Claire and Francesco as reviewers: If you have a moment, we'd like your input on API and refactor intentions (see discussion). |
|
Eventually, I haven't looked at the code in detail yet, but I can comment on the high level design. I agree with @marymcglo here, the way it's implemented allows us to make evalml extensible with little code changes. It's really not a lot: more than half is just config schema changes and the new |
|
Thanks for the input, both! Jonas, I think your gut reaction of this being a large change, is a good indicator that the implementation can be made less awkward. Especially since Francesco is suggesting that we won't be separating out inference in the next refactor (if I'm understanding correctly), let's chat tomorrow about what I can do to make this a bit more maintainable. Thanks for the feedback! |
This is exactly what motivated my initial comment. I don't see why we wouldn't want to open up evalml to process bring-your-own GRIB (btw very much like the name @marymcglo !) as a first-class use case. In my view this would simplify the rules and avoid the implicit assumption and limitation that forecasts without grib are always baselines, that has to now fixed by bypassing inference to allow evaluation of hirad to work. Maybe the use-case of evaluation baselines against each other is not the most urgent. More likely we will want to evaluate a new ML model against our current operational one soon (without any additional baselines), which is also not possible for all evalml flavours because some just require baselines to be present (e.g. scorecards). In that sense I would still argue we should consider treating BYO grib as a first class citizen and allow any run to become a baseline by explicit configuration. |
|
Ah, thanks for clarifying Jonas! I think I misunderstood your initial suggestion. Jonas and I chatted a bit at the desks, and I started some intial prototyping in #254 to explore other options for how to implement GRIB models in a more forward-thinking way. But my brain is already on vacation mode so I haven't put the thought into it I want to yet. :) Since this isn't urgent, I can come back to this after next week. In the meantime, I'll just leave this branch open for Louis to continue development of a config that supports both WeatherGenerator and HiRAD. We should be able to easily change the backend implmentation without needing to change the configs, so it seems fine to stay on this branch. |
…ly re-used for WeatherGenerator, until there is reason to differentiate otherwise.
No description provided.