Skip to content

fix: Inference with crps requires additional arguments in predict_step #212

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 28, 2025

Conversation

jakob-schloer
Copy link
Contributor

Description

The problem is described in ecmwf/anemoi-core#277. The forward function of AnemoiEnsModelEncProcDec takes additional arguments that need to be passed from anemoi-inference.

@FussyDuck
Copy link

FussyDuck commented Apr 25, 2025

CLA assistant check
All committers have signed the CLA.

ssmmnn11
ssmmnn11 previously approved these changes Apr 25, 2025
Copy link
Member

@ssmmnn11 ssmmnn11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgt :-)

@HCookie
Copy link
Member

HCookie commented Apr 25, 2025

@jakob-schloer
Copy link
Contributor Author

We have a CRPS runner for this, no? https://github.com/ecmwf/anemoi-inference/blob/main/src/anemoi/inference/runners/crps.py

Good to know. Do you need to change the config for that? Simply running anemoi-inference run did not work out of the box for me.

@HCookie
Copy link
Member

HCookie commented Apr 28, 2025

Yes, you'll need to use

runner: crps

@ssmmnn11
Copy link
Member

But the runner was a work-around to be able to run with the old branch @gmertes

Copy link
Member

@gmertes gmertes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with this, the idea being that every (future) extra to predict step has to be a keyword argument and not a positional.

@HCookie what do you think?

@mchantry mchantry merged commit 7917d53 into main Apr 28, 2025
68 checks passed
@mchantry mchantry deleted the fix/inference-with-crps branch April 28, 2025 15:09
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.

6 participants