Conversation
egparedes
added this pull request to stack #2900
September 22, 2026 07:00
egparedes
force-pushed
the
connectivities-as-types-6-class-keyed-providers
branch
from
September 23, 2026 16:10
2ec01e1 to
6a5a914
Compare
egparedes
force-pushed
the
connectivities-as-types-7-constlist-axisliteral
branch
from
September 23, 2026 16:10
2898820 to
f468726
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
Reductions, sparse arguments and list materialization used to find a connectivity by the local dimension's tag, which only names the owner's table. They now look up a table over the local dimension (common.connectivity_key_over), so a connectivity sharing another one's local dimension works on every backend, bound on its own or together with its owner. DaCe sizes a connectivity array's local dimension from its own table. Lifts the PR 1 xfail markers.
- iterator tracing and embedded shift name a sharing connectivity by offset_tag - DaCe if/scan/concat_where/const-list sites find the table over the local dimension - connectivity_key_over takes a tag, avoids resolve, and picks deterministically - embedded map_list compares lists by local dimension, not by offset - a sharer must have its owner's origin; counts are compared one by one - ADR 0029: sharers must have the owner's neighbor structure
Offset providers are keyed by NeighborConnectivity declarations. Every program entry point normalizes them to the tag-keyed form the IR and the backends use (as_tag_keyed_offset_provider), and tables are checked against their declarations once per compiled variant and on embedded calls (check_offset_provider). A bare string key is rejected as the removed FieldOffset spelling. FieldOffset is removed: unstructured connectivities are NeighborConnectivity declarations, Cartesian shifts are 'Dim + i', and as_offset takes the dimension to shift along. scripts/python/migrate_connectivities.py migrates user code.
- resolve memoizes only where the module path ends, so a redefined declaration (a re-run notebook cell) resolves to the new class; mismatch messages say so - normalize the provider in DaCe get_sdfg_conn_args and FieldOperatorFromFoast - compare skip positions of shared local dimensions on the tables' device - migration script: import aliases, unqualified names (imports rewritten), bare LOCAL, shadowed names, __all__, tags differing from the variable, Cartesian provider keys reported as removable - drop the working plan document committed by mistake; clear a stale notebook output showing a removed spelling
- check the provider at the entry points that only normalized it: the iterator 'fendef', 'FieldOperatorFromFoast' and the DaCe orchestration's connectivities - remember a checked provider by the identity of its tables, and read the tables (comparing skip positions of shared local dimensions) only where a program is compiled, not on the call path - reject a key that names the connectivity rather than the declaration - cache a key that is not a qualified name, so it costs one import attempt - DaCe 'get_sdfg_conn_args' is an IR-level hook, so it is not strict either - the migration script writes 'Local: TypeAlias = ...' and imports 'typing' - PR 1's bare-Cartesian-offset check is unreachable without 'FieldOffset' - ADR 0029: which entry points are strict, and what the checks read
- common.ConstList: the owner-less, size-1 local dimension of make_const_list, used directly by iterator embedded and the DaCe lowering (identity checks) - AxisLiteral stores only the tag; kind and dim are resolved from it. The pretty printer derives the suffix (now with ₗ for local dimensions, which used to be printed as vertical), and the parser ignores it.
The pretty printer takes an axis literal's kind from its inferred type, or from a dimension that is already loaded (common.resolve_loaded), and never imports. gtfn's domain canonicalization resolves through dim_from_axis_literal; ADR 0028 states what resolve memoizes.
- rename the references the earlier PRs added along with the class - ADR 0028's date, which this PR's edits had outrun - say why 'ListType.offset_type' can be 'None' while embedded uses 'ConstList' - test 'resolve_loaded', which is what keeps printing IR import-free
egparedes
force-pushed
the
connectivities-as-types-6-class-keyed-providers
branch
from
September 23, 2026 16:54
6a5a914 to
0619aca
Compare
egparedes
force-pushed
the
connectivities-as-types-7-constlist-axisliteral
branch
from
September 23, 2026 16:54
f468726 to
36cbcb7
Compare
egparedes
force-pushed
the
connectivities-as-types-6-class-keyed-providers
branch
from
September 24, 2026 10:12
0619aca to
4ea2cfd
Compare
Contributor
Author
|
Absorbed into #2899: the stack was shortened from 7 PRs to 4, and this PR's commits (ConstList replacing _CONST_DIM, AxisLiteral without a stored kind, IR printing without imports, doctest dimension registration) now sit on top of #2899's own commits. The one part that needs a later concept, ConstList as a LocalDimensionIndex that cannot be adopted, is in #2907. |
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 #2910 (PR 6). Part 7 of the connectivities as types stack.
What
Two cleanups that the class-based dimensions make possible:
common.ConstListis the local dimension ofmake_const_listresults: an owner-lessLocalDimensionIndexof size 1. The class itself already arrived in PR 2 asConstListDim; this PR renames it and makes it replace the two_CONST_DIMaliases in iterator embedded and the DaCe lowering, whose checks now use identity (is). Belonging to no connectivity, it cannot be adopted as a declaration'sLocal.AxisLiteralstores only the tag.AxisLiteral.kindand the newAxisLiteral.dimare properties resolved from the tag, which removes theTODOatiterator/ir.py. With the kind derived rather than stored, it can no longer disagree with the dimension's own. It already did in one case: the pretty printer printed a local axis as vertical (ᵥ), so a local axis didn't round-trip through the textual IR. The printer now derives the suffix (ₕ,ᵥ, and the newₗ), and the parser ignores it.__str__. The derived properties go through the newcommon.resolve_loaded, which only looks insys.modulesand returnsNoneotherwise.Every
AxisLiteral(value=..., kind=...)construction insrcandtestsloses itskind=.Verification
unit + integration + regression 6152 passed / 0 failed (
-n 2, 46 min); notebooks (slides, exercise solutions, Quickstart, advanced, examples) 19 passed;src/gt4py/nextdoctests 119 passed; typing tests 20 mypy cases on 3.12/3.13/3.14 plus a pyright run overtyping_tests/pyright_probes.py(0 errors); migration-script tests 6 passed;pre-commit run --all-filesclean.