Skip to content

Add feature to evaluate experimental models that produce their own GRIB files (e.g. HiRAD, WeatherGenerator) - #253

Open
marymcglo wants to merge 9 commits into
testing-hackathonfrom
feat/hirad-integration
Open

Add feature to evaluate experimental models that produce their own GRIB files (e.g. HiRAD, WeatherGenerator)#253
marymcglo wants to merge 9 commits into
testing-hackathonfrom
feat/hirad-integration

Conversation

@marymcglo

Copy link
Copy Markdown
Collaborator

No description provided.

@marymcglo
marymcglo requested a review from jonasbhend August 18, 2026 15:43
@jonasbhend

Copy link
Copy Markdown
Contributor

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)?

@marymcglo

marymcglo commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator Author

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 evalml experiment) as a first-class use case or not.

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.

@marymcglo marymcglo self-assigned this Aug 19, 2026
@marymcglo

Copy link
Copy Markdown
Collaborator Author

Added Claire and Francesco as reviewers: If you have a moment, we'd like your input on API and refactor intentions (see discussion).

@frazane

frazane commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Eventually, evalml will generally not be responsible for running inference. Users will specify something that will point to existing forecasts. But I don't think we are ready yet.

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 hirad.yaml. I don't see how this impacts the rest of the workflow in any way, it seems pretty orthogonal. It simply formalizes the fact that it's possible to provide external sources as long as the directory structure in the root is the one that evalml expects.

@marymcglo

Copy link
Copy Markdown
Collaborator Author

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!

@jonasbhend

Copy link
Copy Markdown
Contributor

I think this hits on the fundamental question of whether we want to support BYO GRIB (or more generally, skipping inference in evalml experiment) as a first-class use case or not.

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.

@marymcglo

Copy link
Copy Markdown
Collaborator Author

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.

Mary McGlohon added 3 commits August 20, 2026 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants