Skip to content

feat[next]: allow runtime shift offsets, and use them for the SWM periodic halo - #2750

Draft
havogt wants to merge 1 commit into
havogt/swm-example-notebookfrom
havogt/swm-periodicity
Draft

feat[next]: allow runtime shift offsets, and use them for the SWM periodic halo#2750
havogt wants to merge 1 commit into
havogt/swm-example-notebookfrom
havogt/swm-periodicity

Conversation

@havogt

@havogt havogt commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Stacked on #2749. Moves the shallow water model's periodic halo update into the DSL, replacing the NumPy halo exchange that PR 1 deliberately used to stay free of src/ changes.

The example side

@gtx.field_operator
def make_periodic(f: IJField, m: gtx.int32, n: gtx.int32) -> IJField:
    f = concat_where(I == -1, f(I + m), f)
    f = concat_where(I == m,  f(I - m), f)
    f = concat_where(J == -1, f(J + n), f)
    f = concat_where(J == n,  f(J - n), f)
    return f

timestep now returns the wrapped prognostic fields, and the time loop touches no halo at all. The prognostic outputs are written over the full domain while the filtered levels only need the interior, so a program supplies a per-output domain=.

The rebinding order is load-bearing: each concat_where feeds the next, so the j pass sees the already-i-corrected field and the corners come out right. A simultaneous formulation would leave them wrong.

Why this needs frontend changes

The shift distance is m, a program argument rather than a literal, and the frontend rejects that at FOAST construction:

DSLError: Cartesian offsets are only supported with a literal right-hand side,
          e.g. 'IDim + 1', but not 'IDim + expr'.

with_static_args cannot substitute — the check fires when the decorator runs at import, long before compile(m=[M]) exists.

Four files, and an ablation confirms none is redundant. Each row is measured by reverting that change alone and running all three backends:

change required for
ffront/foast_passes/type_deduction.py all backends — otherwise DSLError at import
ffront/foast_to_gtir.py gtfn + dace (embedded never lowers to GTIR)
iterator/transforms/normalize_shifts.pyCanonicalizeShiftOffsets gtfn + dace
pass_manager.py, wiring in apply_common_transforms gtfn only
pass_manager.py, wiring in apply_fieldview_transforms dace only

The last two lines are the non-obvious part: the two backends traverse different pass managers, so each wiring serves exactly one. Dropping either gives AttributeError: 'DomainAccessDescriptor' object has no attribute 'grid_type' on the corresponding backend.

CanonicalizeShiftOffsets exists because our own lowering emits a shift whose offset is a runtime expression; once m is inlined as a static argument it becomes an ir.Literal, but domain inference expects an ir.OffsetLiteral and otherwise yields the unknown-access sentinel.

Validation

  • notebook, 100 steps vs the NumPy reference — passes
  • notebook, full 4000 steps vs swm_reference.npz (NCAR data) — run manually, passes
  • pytest --nbmake examples — passes

Numerically identical to PR 1: the NumPy reference is unchanged and still agrees, so moving the halo update into the DSL did not perturb the physics.

Unit suite

pytest tests/next_tests/unit_tests/ compared against the same suite on #2749's branch, since main already carries pre-existing failures:

failures + errors
baseline (#2749 branch) 19
this PR, before updating the spec test 20
this PR 19

The one difference was test_foast_to_gtir.py::test_premap_cartesian_non_literal_offset, which asserted that inp(TDim + i) raises. That is the behaviour this PR deliberately changes, so the test is updated rather than silenced — it now pins the lowering, plus two new cases: the SUB form (neg applied to the offset) and that a non-literal float offset still errors, since a runtime offset has no fractional part to resolve to a staggered dimension.

Failure sets are otherwise identical (comm on the sorted lists shows no other difference in either direction).

Note

An alternative that needs no src/ changes is to hardcode the wrap distance as a literal. I measured it and rejected it: it silently miscomputes at any --M other than the default, because the domain follows the grid size but the wrap does not.

Cartesian shift offsets had to be literals, which meant a periodic wrap by a
runtime grid size could not be written in the DSL. Relax the frontend to accept
a non-literal integral offset, lower it, and canonicalize the resulting literal
to an OffsetLiteral so domain inference sees a real offset rather than the
unknown-access sentinel.

Uses it in the shallow water example, whose halo update moves from NumPy into
the field operator. Updates test_premap_cartesian_non_literal_offset, which
pinned the old rejection, and adds the SUB and non-integral cases.
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