Skip to content

feat[next]: declare connectivities as classes; FieldOffset derived from them - #2908

Closed
egparedes wants to merge 7 commits into
connectivities-as-types-3-neighbor-connectivityfrom
connectivities-as-types-4-declare-connectivities
Closed

egparedes wants to merge 7 commits into
connectivities-as-types-3-neighbor-connectivityfrom
connectivities-as-types-4-declare-connectivities

Conversation

@egparedes

Copy link
Copy Markdown
Contributor

Stacked on #2907 (PR 3). Part 4 of the connectivities as types stack (ADR 0028, ADR 0029).

What

The tree switches from FieldOffset + separately declared local dimensions to connectivity declarations:

# before
class V2EDim(gtx.DimensionIndex, kind=gtx.DimensionKind.LOCAL): ...
V2E = gtx.FieldOffset(V2EDim.tag, source=Edge, target=(Vertex, V2EDim))

# after
class V2E(gtx.NeighborConnectivity[Vertex, Edge]):
    class Local(gtx.LocalDimensionIndex): ...
V2EDim = V2E.Local  # only where existing code keeps using the old name
  • Fixtures and tests: toy_connectivity.py (V2E, E2V, C2E, V2V), cases_utils.py (imports them and declares C2V), fvm_nabla_setup.py, and the unit tests that declared their own offsets. The provider keys X.value become X.Local.tag. PR 2 already wrote the ITIR-level strings symbolically as XDim.tag, so they stay valid with no edits.
  • Docs: the Quickstart guide (declares E2C/C2E, uses E2C.Local), workshop helpers.py, and slides_2.
  • Local dimensions must subclass LocalDimensionIndex. A DimensionIndex subclass declared with kind=DimensionKind.LOCAL is now a TypeError that names the replacement, so a local dimension always has an owner slot. ConstListDim moves under LocalDimensionIndex.
  • The two-target FieldOffset is deprecated, with a DeprecationWarning pointing at NeighborConnectivity. The offsets derived from a declaration don't warn. Cartesian FieldOffsets (used only by as_offset) stay until PR 6.
  • Iterator-level embedded shift/neighbors and iterator tracing accept a connectivity class.

FieldOffset stays in a few tests that exercise the legacy path until PR 6 removes it.

Verification

unit 2256 passed, integration + regression 3803 passed / 0 failed (-n 2), notebooks (slides, exercise solutions, Quickstart, advanced, examples) all pass, src doctests pass, pre-commit --all-files clean.

Review

An independent adversarial review approved this PR. Its one design point was connectivities that share a local dimension, like ICON4Py's C2CE. They are now supported by the last commit of #2907, and PR 5 adds the backend support.

@egparedes
egparedes added this pull request to stack #2900 September 22, 2026 06:59
@egparedes
egparedes force-pushed the connectivities-as-types-3-neighbor-connectivity branch from d1e1aeb to 8c55110 Compare September 23, 2026 16:09
@egparedes
egparedes force-pushed the connectivities-as-types-4-declare-connectivities branch from 83aad0c to 6de3f56 Compare September 23, 2026 16:09
- check_neighbor_table: min_neighbors exceeding the table, bool tables
- descriptive errors for non-integer indices, the undeclared base, DSL attributes
- FieldOffset.Local, so the spelling works for legacy offsets in embedded too
- fingerprint a declaration by its dimensions and counts, not only its name
- negative counts are a ValueError
…mension

Flattened sparse patterns (ICON4Py's C2CE, E2ECV, ...) index the same neighbor
axis as another connectivity. A declaration can now adopt an owned local
dimension; it is then named in the IR by its own tag (offset_tag), since the
local dimension's tag already names the owner's table.
An annotated 'Local' -- on 'NeighborConnectivity' or on its metaclass -- makes
every declaration's local dimension a *variable*: pyright then rejects
'Field[Dims[V, V2E.Local], float]', and mypy rejects an adopted or shared one.
Annotate it nowhere; library code reads it through 'common.local_dimension_of',
and adoption and sharing are written 'Local: TypeAlias = ...'.

Also from the review: a redefined declaration re-owns an adopted local dimension
instead of becoming a sharer, and its counts are checked against the local
dimension's own 'size='. The missing-'Local' error names both spellings.

Adds pyright over 'typing_tests/pyright_probes.py' to the typing session, which
is what catches a regression here: the mypy cases cannot.
- write an adopted or shared 'Local' as 'Local: TypeAlias = ...', the spelling
  that keeps it a type for mypy as well as pyright
- refuse to adopt the 'make_const_list' local dimension, which now is one
- drop a stale commented-out string-keyed offset provider
@egparedes
egparedes force-pushed the connectivities-as-types-3-neighbor-connectivity branch from 8c55110 to bf5c950 Compare September 23, 2026 16:54
@egparedes
egparedes force-pushed the connectivities-as-types-4-declare-connectivities branch from 6de3f56 to 7f1b5c8 Compare September 23, 2026 16:54
@egparedes
egparedes force-pushed the connectivities-as-types-3-neighbor-connectivity branch from bf5c950 to 90252b3 Compare September 24, 2026 10:12
@egparedes

Copy link
Copy Markdown
Contributor Author

Absorbed into #2910: the stack was shortened from 7 PRs to 4. Declaring the tree's connectivities as classes now lands together with class-keyed offset providers and the removal of FieldOffset, as a single migration of the tree; the intermediate FieldOffset-derived-from-a-declaration shim and its DeprecationWarning are not carried over.

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.

1 participant