Skip to content

Conversation

@HCookie
Copy link
Member

@HCookie HCookie commented Nov 5, 2025

Description

Add anemoi-(graphs & models) to dependencies

What issue or task does this change relate to?

Closes #350

As a contributor to the Anemoi framework, please ensure that your changes include unit tests, updates to any affected dependencies and documentation, and have been tested in a parallel setting (i.e., with multiple GPUs). As a reviewer, you are also responsible for verifying these aspects and requesting changes if they are not adequately addressed. For guidelines about those please refer to https://anemoi.readthedocs.io/en/latest/

By opening this pull request, I affirm that all authors agree to the Contributor License Agreement.

@HCookie HCookie self-assigned this Nov 5, 2025
@github-project-automation github-project-automation bot moved this to To be triaged in Anemoi-dev Nov 5, 2025
@github-actions github-actions bot added dependencies Pull requests that update a dependency file and removed dependencies Pull requests that update a dependency file labels Nov 5, 2025
@bdvllrs
Copy link
Contributor

bdvllrs commented Nov 6, 2025

Hi! Thanks for this. So no minimum version?

@HCookie
Copy link
Member Author

HCookie commented Nov 6, 2025

@bdvllrs Not at the moment, partially as I have no idea what it would be.
As we update models, we can revisit this

frazane
frazane previously approved these changes Nov 6, 2025
@HCookie HCookie moved this from To be triaged to Under Review in Anemoi-dev Nov 6, 2025
@github-actions github-actions bot added the dependencies Pull requests that update a dependency file label Nov 6, 2025
@HCookie HCookie changed the title fix: Add anemoi-(graphs & models) to dependencies fix: Add anemoi-models to dependencies Nov 7, 2025
@frazane
Copy link
Contributor

frazane commented Nov 19, 2025

I guess what's missing from this PR is some guardrails. Personally, I would opt for a check as soon as the checkpoint is loaded, where we assert that the version of anemoi-models currently installed in the python environment is the same as the one specified by the checkpoint's requirements. If it's not, then an error is raised, unless a flag is passed to the config, in which case only a warning is logged. What do you think @HCookie?

@anaprietonem
Copy link
Contributor

anaprietonem commented Nov 20, 2025

I guess what's missing from this PR is some guardrails. Personally, I would opt for a check as soon as the checkpoint is loaded, where we assert that the version of anemoi-models currently installed in the python environment is the same as the one specified by the checkpoint's requirements. If it's not, then an error is raised, unless a flag is passed to the config, in which case only a warning is logged. What do you think @HCookie?

@frazane how would this be different then than the current workflow in main? - where one can run the inspect command get the version of models needed - install it and then run inference? - as in what would you win with this new approach if at the end installing anemoi-models need to be reviewed and checked by the user

@frazane
Copy link
Contributor

frazane commented Nov 20, 2025

I guess what's missing from this PR is some guardrails. Personally, I would opt for a check as soon as the checkpoint is loaded, where we assert that the version of anemoi-models currently installed in the python environment is the same as the one specified by the checkpoint's requirements. If it's not, then an error is raised, unless a flag is passed to the config, in which case only a warning is logged. What do you think @HCookie?

@frazane how would this be different then than the current workflow in main? - where one can run the inspect command get the version of models needed - install it and then run inference? - as in what would you win with this new approach if at the end installing anemoi-models need to be reviewed and checked by the user

There is no “win” in terms of workflow changes - it’s about correctness and clarity. If a package cannot perform its core functionality without another package installed, then that package is a required runtime dependency and should be declared as such. The fact that adding guardrails would lead to the same practical workflow as on main is, from my perspective, actually an argument in favour of this proposal.

Maybe I am being too rigid about this, I don't know. I agree that this discussion is of little practical importance, but I do think that the principle matters...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ATS Approval needed dependencies Pull requests that update a dependency file

Projects

Status: Under Review

Development

Successfully merging this pull request may close these issues.

Should anemoi-models be added to the dependencies?

5 participants