Skip to content

Conversation

@JPXKQX
Copy link
Member

@JPXKQX JPXKQX commented Oct 22, 2025

Description

Instead of using ICONNodes, decouple the different parts to be computed in each node and edge builder.

nodes:
  # Data nodes
  data:
    node_builder:
      _target_: anemoi.graphs.nodes.ICONCellGridNodes
      max_level: 3
      grid_filename: icon_grid_0026_R03B07_subsetAICON.nc
  # Hidden nodes
  hidden:
    node_builder:
      _target_: anemoi.graphs.nodes.ICONMultimeshNodes
      max_level: 3
      grid_filename: icon_grid_0026_R03B07_subsetAICON.nc
edges:
  # Processor configuration
  - source_name: hidden
    target_name: hidden
    edge_builders:
      - _target_: anemoi.graphs.edges.ICONTopologicalProcessorEdges
  - source_name: data
    target_name: hidden
    edge_builders:
      - _target_: anemoi.graphs.edges.ICONTopologicalEncoderEdges
  - source_name: hidden
    target_name: data
    edge_builders:
      - _target_: anemoi.graphs.edges.ICONTopologicalDecoderEdges

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.


📚 Documentation preview 📚: https://anemoi-training--627.org.readthedocs.build/en/627/


📚 Documentation preview 📚: https://anemoi-graphs--627.org.readthedocs.build/en/627/


📚 Documentation preview 📚: https://anemoi-models--627.org.readthedocs.build/en/627/

@github-project-automation github-project-automation bot moved this to To be triaged in Anemoi-dev Oct 22, 2025
@JPXKQX JPXKQX self-assigned this Oct 22, 2025
@JPXKQX JPXKQX requested a review from MeraX October 22, 2025 12:45
@JPXKQX JPXKQX added the ATS Approval Not Needed No approval needed by ATS label Oct 22, 2025
@JPXKQX JPXKQX marked this pull request as ready for review October 25, 2025 13:03
@fprill
Copy link
Contributor

fprill commented Nov 6, 2025

Thanks! Cleaning up and refactoring is always nice!
A few questions:

  • Setting for grid_filename in both, data and hidden: Don't you think that this could allow to use inconsistent ICON meshes for the encoder/decoder and the hidden mesh? The edge builder would fail then.
  • Minor things: "ICONMultiMeshNodes": Why did you decide to use this spelling? GraphCast writes this as "multi-mesh"... Yes, I know that the previous implementation wasn't consistent :)
  • nodeset: NodeSet # set of ICON cell circumcenters - Why does the PR remove this comment? Just a matter of taste, but I also think that a comment like "duplicate edges (bi-directional)" is helpful for some readers....
  • Why was the .to(device) necessary (has been added in various places)?
  • A question: Is there any danger that the BaseICONNodeBuilder will ever be initialized after the EdgeBuilder?
  • Again, thanks for cleaning up. But, regarding the GeneralGraph and the BipartiteGraph: I actually liked these abstract base classes which separate the "ICON stuff" from some general "graph stuff". But I would also agree with your opinion if you have an argument against this mini-hierarchy.

@HCookie HCookie moved this from To be triaged to Under Review in Anemoi-dev Nov 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Under Review

Development

Successfully merging this pull request may close these issues.

3 participants