Conversation
egparedes
added this pull request to stack #2900
September 22, 2026 06:59
egparedes
force-pushed
the
connectivities-as-types-3-neighbor-connectivity
branch
from
September 23, 2026 16:09
d1e1aeb to
8c55110
Compare
egparedes
force-pushed
the
connectivities-as-types-4-declare-connectivities
branch
from
September 23, 2026 16:09
83aad0c to
6de3f56
Compare
…at know their owner
- 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
force-pushed
the
connectivities-as-types-3-neighbor-connectivity
branch
from
September 23, 2026 16:54
8c55110 to
bf5c950
Compare
egparedes
force-pushed
the
connectivities-as-types-4-declare-connectivities
branch
from
September 23, 2026 16:54
6de3f56 to
7f1b5c8
Compare
egparedes
force-pushed
the
connectivities-as-types-3-neighbor-connectivity
branch
from
September 24, 2026 10:12
bf5c950 to
90252b3
Compare
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. |
egparedes
removed this pull request from stack #2900
September 24, 2026 10:14
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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: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 keysX.valuebecomeX.Local.tag. PR 2 already wrote the ITIR-level strings symbolically asXDim.tag, so they stay valid with no edits.E2C/C2E, usesE2C.Local), workshophelpers.py, andslides_2.LocalDimensionIndex. ADimensionIndexsubclass declared withkind=DimensionKind.LOCALis now aTypeErrorthat names the replacement, so a local dimension always has anownerslot.ConstListDimmoves underLocalDimensionIndex.FieldOffsetis deprecated, with aDeprecationWarningpointing atNeighborConnectivity. The offsets derived from a declaration don't warn. CartesianFieldOffsets (used only byas_offset) stay until PR 6.shift/neighborsand iterator tracing accept a connectivity class.FieldOffsetstays 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-filesclean.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.