-
Notifications
You must be signed in to change notification settings - Fork 78
refactor(graphs): Expose Python API #772
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
base: main
Are you sure you want to change the base?
Conversation
|
anemoi-graphs3ee1ba8 to
aab0b02
Compare
5c4021a to
5571316
Compare
3ab235a to
0c6d2da
Compare
170e9fe to
84a8fb0
Compare
for more information, see https://pre-commit.ci
|
|
||
| import numpy as np | ||
| import torch | ||
| from hydra.utils import instantiate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is being removed, do we expect the yaml generated as a result of the API generation to work? I think a test checking that would be ideal but just wondering if a first step, you had tried to generate the graph with the config and that still works, and same for training?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is currently no YAML generated from the GraphBuilder (i. e the pure python API - ideas for a better name welcome). GraphCreator can still be instantiated with a yaml config as before. I have checked locally that the graphs created by the two are the same on a few cases but I agree I should add some tests to the parsers and to check that graphs are the same.
|
@radiradev - current configs for graphs have interconnections with for example the dataloader section https://github.com/ecmwf/anemoi-core/blob/main/training/src/anemoi/training/config/graph/multi_scale.yaml how would this be handled by the API? - I think this a useful feature but one that I think during the working group session in December we also touched on the fact that it brings some additional complexity, so not flagging this a must that we would need to keep, but rather curious on the interplay of the two features. |
@anaprietonem currently the |
!fdl.Config
__fn_or_cls__:
module: anemoi.graphs.create
name: GraphBuilder
edges:
- !fdl.Config
__fn_or_cls__:
module: anemoi.graphs.edges.builders.cutoff
name: CutOffEdges
attributes:
- !fdl.Config
__fn_or_cls__:
module: anemoi.graphs.edges.attributes
name: EdgeLength
norm: unit-std
- !fdl.Config
__fn_or_cls__:
module: anemoi.graphs.edges.attributes
name: EdgeDirection
norm: unit-std
cutoff_factor: 0.6
source_name: data
target_name: hidden
... |
|
I've made some suggestions to your PR in radiradev#3, hopefully they are useful! |
Description
This PR introduces an alternative way to construct graphs by instantiating objects directly, in addition to the existing YAML-based configuration approach.
Creating graphs via direct object instantiation improves transparency: docstrings are visible, IDE enables us to “go to definition”, and objects can be inspected directly during development. Furthermore this way we can utilise static type checking, reducing the need for Pydantic schema validation.
Sidenote: The GraphCreator could already be instantiated directly on
mainin Python, but this requires to pass in aDotDictwhich is basically a parsed config. This PR instead explores using the edge, node and attribute objects directly.This is my first attempt as part of the broader plan in #768 and would serve as a test-bed for ideas and general feedback to a more pythonic
anemoi.Example graph with the new Python API:
YAMLequivalentWhat problem does this change solve?
This an experimental new feature that is fully backwards compatible with the
YAMLconfigurationWhat issue or task does this change relate to?
#768
Additional notes
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.