Unify hierarchical mesh edge level metadata by SIDDHANTCOOKIE · Pull Request #66 · mllam/weather-model-graphs

SIDDHANTCOOKIE · 2026-02-21T01:08:12Z

Description

Hey @leifdenby @observingClouds @joeloskarsson
This PR standardizes hierarchical mesh edge level metadata to avoid mixed edge attribute keys/types that can cause downstream conversion issues.

Main changes:

Replace inconsistent edge metadata (level on same-level edges vs levels /on cross-level edges) with a consistent schema on all hierarchical meshedges:
- from_level: int
- to_level: int
- direction: str (existing, retained)
- connection: str (derived, e.g. "1>0")
Remove legacy edge keys (level, levels) from hierarchical mesh edges.
Update hierarchical handling in graph creation logic to identify bottom-level
same-level mesh edges using direction/from_level/to_level.
Update archetype docstring to document the new edge metadata.
Add regression test test_hierarchical_edges_have_consistent_level_metadata
to verify:
- required new keys exist
- types are consistent
- same edges satisfy from_level == to_level
- legacy keys are absent

Issue Link

Fixes issue #45

Type of change

🐛 Bug fix (non-breaking change that fixes an issue)
✨ New feature (non-breaking change that adds functionality)
💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
📖 Documentation (Addition or improvements to documentation)

Checklist before requesting a review

My branch is up-to-date with the target branch - if not update your fork with the changes from the target branch (use pull with --rebase option if possible).
I have performed a self-review of my code
For any new/modified functions/classes I have added docstrings that clearly describe its purpose, expected inputs and returned values
I have placed in-line comments to clarify the intent of any hard-to-understand passages of my code
I have updated the documentation to cover introduced code changes
I have added tests that prove my fix is effective or that my feature works
I have given the PR a name that clearly describes the change, written in imperative form (context).
I have requested a reviewer and an assignee (assignee is responsible for merging)

Checklist for reviewers

Each PR comes with its own improvements and flaws. The reviewer should check the following:

the code is readable
the code is well tested
the code is documented (including return types and parameters)
the code is easy to maintain

Author checklist after completed review

I have added a line to the CHANGELOG describing this change, in a section
reflecting type of change (add section where missing):
- added: when you have added new functionality
- changed: when default behaviour of the code has been changed
- fixes: when your contribution fixes a bug

Checklist for assignee

PR is up to date with the base branch
the tests pass

author has added an entry to the changelog (and designated the change as added, changed or fixed)
Once the PR is ready to be merged, squash commits and merge the PR.

observingClouds · 2026-02-21T06:12:58Z

Thanks @SIDDHANTCOOKIE for this PR. Please use the entire PR template, it helps us to keep a better overview about the process.

SIDDHANTCOOKIE · 2026-02-21T06:43:18Z

@observingClouds Thanks for the clarification, I've updated the description with everything that was missing.

SIDDHANTCOOKIE · 2026-02-21T07:50:25Z

Hey @observingClouds CI notebook was failing due to the earlier metadata rename.
I pushed a follow-up update to docs.

SIDDHANTCOOKIE · 2026-02-24T17:26:41Z

@observingClouds how should I proceed with this?

observingClouds · 2026-02-24T22:13:16Z

Hi @SIDDHANTCOOKIE thanks for asking. Please document the changes in the CHANGELOG.md and we will put this PR on the milestone for the next release

SIDDHANTCOOKIE · 2026-02-25T16:53:42Z

Hey @observingClouds I've added the entry to CHANGELOG.md. Please let me know if any changes are needed.

Also, I was curious on how to contribute further. Should I wait for existing issues to be assigned, or should I open new ones as I discover improvements in the codebase? If there are other repositories in the MLLAM ecosystem that currently need help, I’d be happy to contribute there as well.

Looking forward to your guidance. Thank you!

src/weather_model_graphs/create/mesh/kinds/hierarchical.py

joeloskarsson · 2026-02-26T20:02:43Z

src/weather_model_graphs/create/mesh/kinds/hierarchical.py

+            data.pop("level", None)
+            data.pop("levels", None)


(Same as earlier) Do we actually add these keys now? Or why do we need to drop them here instead of not adding them to begin with?

Thanks for pointing this out, those data.pop("level", None) / data.pop("levels", None) For G_up(hierarchial.py), we copy edge metadata from G_down, in current flow G_down is already normalized, so these keys should not be present. So yes, these two pop lines are redundant now, and I can remove them to keep the code simpler.

Yea if they do nothing now I think you can drop them

joeloskarsson · 2026-02-26T20:07:14Z

src/weather_model_graphs/create/base.py

+                            first_level_graph = split_graph_by_edge_attribute(
+                                level_subgraph, "level"
+                            )[0]


Here we are splitting on "level" again. Will this work?

Yes, this works.

That split on "level" is only in the non-hierarchical fallback branch (when edges do not have from_level/to_level). Hierarchical graphs take the branch above and use direction + from_level/to_level, so they don’t rely on "level" there.

Also @joeloskarsson I forgot to run precommit in my last commit that caused the CI to fail. I've resolved it in my latest commit

joeloskarsson · 2026-02-27T20:54:53Z

Thinking a bit more about this, does it make sense to restrict the scope of this to only hierarchical graphs? Or are we creating another inconsistency now between hierarchical graphs and flat (potentially multi-scale graphs)? In my view we could always view a flat graph as a hierarchical one with from_level=1, to_level=1, direction=same. But now the flat graphs will still have this level attribute. Interested to hear @observingClouds view.

I am thinking that it would make sense to make these attributes consistent across all graphs really.

SIDDHANTCOOKIE · 2026-02-27T21:22:09Z

I agree, @joeloskarsson . Restricting this to hierarchical graphs leaves us with two schemas (from_level/to_level vs level) and that’s not ideal.

I can extend this so all mesh edges use a consistent schema:

from_level
to_level
direction
optional connection

For flat/flat-multiscale, this would be:

direction="same"
from_level=to_level= (or fixed 0 for single-level flat, depending on convention)

leifdenby · 2026-02-27T22:11:16Z

CHANGELOG.md


 ### Fixed
-
+- Fix issue #45 about inconsistent edge metadata and improves compatibility


It would be helpful if you summarised what the attributes that have been removed are and what they have been replaced with

leifdenby · 2026-02-27T22:14:49Z

For flat/flat-multiscale, this would be:

* `direction`="same"

* `from_level=to_level`= (or fixed 0 for single-level flat, depending on convention)

Maybe we just do away with any notion of level for the flat/flat-multiscale meshes? For flat-multiscale this is only the because the multiscale mesh is built by collapsing the individual graphs representing each level of refinement, but we could remove it once we know we're not trying to build a hierarchical mesh?

leifdenby · 2026-03-18T14:53:53Z

@SIDDHANTCOOKIE are you still looking at this?

SIDDHANTCOOKIE · 2026-03-18T15:23:12Z

Hey so sorry I had my uni exams and totally forgot about it can you please tell me what should be the next steps

leifdenby · 2026-03-20T13:49:06Z

Hey so sorry I had my uni exams and totally forgot about it can you please tell me what should be the next steps

No worries! You should absolutely concentrate foremost on your exams!

Once you are ready the first steps would be to engage with the ongoing discussions :) For example the question I asked in #66 (comment), and this one #66 (comment).

Thank you!

Unify hierarchical mesh edge level metadata

98ef11b

Update docs for hierarchical edge metadata schema

c8eb1b9

update changelog

f387b31

joeloskarsson reviewed Feb 26, 2026

View reviewed changes

src/weather_model_graphs/create/mesh/kinds/hierarchical.py Show resolved Hide resolved

joeloskarsson reviewed Feb 26, 2026

View reviewed changes

Fix pre-commit lint issue

fe7ef4d

SIDDHANTCOOKIE requested a review from joeloskarsson February 26, 2026 21:01

removed the lines

42ad2dc

leifdenby reviewed Feb 27, 2026

View reviewed changes


		### Fixed

		- Fix issue #45 about inconsistent edge metadata and improves compatibility

Conversation

SIDDHANTCOOKIE commented Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issue Link

Type of change

Checklist before requesting a review

Checklist for reviewers

Author checklist after completed review

Checklist for assignee

Uh oh!

observingClouds commented Feb 21, 2026

Uh oh!

SIDDHANTCOOKIE commented Feb 21, 2026

Uh oh!

SIDDHANTCOOKIE commented Feb 21, 2026

Uh oh!

SIDDHANTCOOKIE commented Feb 24, 2026

Uh oh!

observingClouds commented Feb 24, 2026

Uh oh!

SIDDHANTCOOKIE commented Feb 25, 2026

Uh oh!

Uh oh!

joeloskarsson Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

SIDDHANTCOOKIE Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

joeloskarsson Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

joeloskarsson Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

SIDDHANTCOOKIE Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

SIDDHANTCOOKIE Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

joeloskarsson commented Feb 27, 2026

Uh oh!

SIDDHANTCOOKIE commented Feb 27, 2026

Uh oh!

leifdenby Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

leifdenby commented Feb 27, 2026

Uh oh!

leifdenby commented Mar 18, 2026

Uh oh!

SIDDHANTCOOKIE commented Mar 18, 2026

Uh oh!

leifdenby commented Mar 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

SIDDHANTCOOKIE commented Feb 21, 2026 •

edited

Loading