Conversation
## Description
We introduced the (strict) support for `IntEnum` in code and arguments
of stencils. The strategy is to replace them with proper integer as soon
as possible.
Guardrails exists for name clash and `IntEnum` derivation.
Valid code looks like this
```python
@gtscript.enum
class LocalEnum(IntEnum):
A = 42
B = 1000
@GTScript.stencil
def enum(field: gtscript.Field[float], order: LocalEnum): # type: ignore
with computation(PARALLEL), interval(0, 1):
if order > LocalEnum.A:
field[0, 0, 0] = LocalEnum.B
```
TO CLEAN
- [ ] Register of Enum types should not leave in `gtscript`
- [x] ADR
- [x] Mixed precision
## Requirements
- [ ] All fixes and/or new features come with corresponding tests.
- [ ] Important design decisions have been documented in the appropriate
ADR inside the [docs/development/ADRs/](docs/development/ADRs/README.md)
folder.
## Description Python 3.14 will remove support for a couple of (long) deprecated `ast` types. Currently produces warnings like ```none DeprecationWarning: ast.Ellipsis is deprecated and will be removed in Python 3.14; use ast.Constant instead ``` or ```none DeprecationWarning: ast.Str is deprecated and will be removed in Python 3.14; use ast.Constant instead ``` This PR updates the (cartesian) gtscript frontend to not use those types anymore and instead rely on things that are future proof, thus eliminating the warning messages. ## Requirements - [x] All fixes and/or new features come with corresponding tests. Covered by existing tests cases. - [ ] Important design decisions have been documented in the appropriate ADR inside the [docs/development/ADRs/](docs/development/ADRs/README.md) folder. N/A
`prune_empty_concat_where` decided whether a branch is ever selected from the branch's own domain, filtered to the dimensions of the condition, and replaced the `concat_where` by the other branch without checking what that branch spans. `concat_where(u⟨ Kᵥ: [-∞, 0[ ⟩, a, b)` with `a: Field[[Vertex, K]]` and `b: Field[[K]]`, accessed on `Vertex: [0, 10[, K: [0, 10[`, was therefore rewritten to `b`, replacing a two dimensional expression by a one dimensional one. Type inference then rejects the program, or -- with assertions disabled -- the mismatch reaches the backend. A `concat_where` spans the dimensions of its condition and of both branches, since a branch is implicitly broadcast to the dimensions it does not have itself. Both branch domains are promoted to those dimensions before intersecting with the region a branch is selected on, and a branch may only replace the entire expression when it spans all of them. Dimensions are read off the branch domains rather than the types, because `infer_domain._infer_concat_where` rebuilds the node through `im.call`, which produces an untyped `FunCall`. The region a branch is selected on is what `_infer_concat_where` already computes. That computation moves to `domain_utils.concat_where_branch_domain` and both callers use it, so the requirement to take the complement before promoting -- `domain_complement` is undefined on a range that is infinite on both sides -- exists in one place.
A branch of a `concat_where` is broadcast to the dimensions it does not have itself, so `concat_where(u⟨ Kᵥ: [-∞, 0[ ⟩, a, b)` with `a: Field[[Vertex]]` and `b: Field[[Vertex, K]]` is the same as `concat_where(u⟨ Kᵥ: [-∞, 0[ ⟩, broadcast(a, (Vertex, K)), b)`. Leaving that broadcast implicit loses the region a branch is selected on: domain inference restricts the domain of an expression to the dimensions of its type, so for `a` above the selected region `Vertex: [0, 10[, K: [0, 0[` is recorded as `Vertex: [0, 10[` -- and it is exactly the dropped range that is empty. A branch that is selected nowhere hence survived `prune_empty_concat_where`, and a branch that is selected everywhere could not replace the expression because it does not span all its dimensions. `broadcast_branches` inserts the broadcast, which `remove_broadcast` turns into an `as_fieldop(deref, ...)` right after pruning -- the same expression that the DaCe lowering of `concat_where` builds for such a branch anyway. For the gtfn pipeline the additional field operator is fused away again.
|
Superseeded by GridTools#2795. Should cover all cases and more. |
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
4 participants
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.
Staged on the fork for discussion — an alternative to GridTools#2767, not a replacement proposal. It implements the design @tehrengruber suggested in review there: prune from the arguments rather than from the
concat_where's own domain, sinceconcat_whereimplicitly broadcasts the dimensions a branch lacks.The finding that shaped it
Pruning from the branch domains alone cannot fix the never-selected-branch bug: the information is not present. Two expressions with byte-identical branch domains require opposite answers, differing only in
node.annex.domain:a.annex.domainb.annex.domainV:[0,10[, K:[0,10[V:[0,10[V:[0,10[, K:[0,10[bV:[0,10[, K:[-5,10[V:[0,10[V:[0,10[, K:[0,10[a: Field[[Vertex]],b: Field[[Vertex, K]], conditionK < 0in both.infer_exprfilters each branch's domain to the dimensions of the branch's own type, and for the branch lacking the concat dimension the one range that would be empty is exactly the one dropped.So the design completes in one of three ways: feed the promotion the
concat_where's domain (that is GridTools#2767), materialise the broadcast in the IR, or leave that bug unfixed. This branch does the second, which is what the review comment writes out asbroadcast(a, (Vertex, K)).The two commits
b04eb80b8— decide pruning from the branches' domains. Computes theconcat_where's dimension set ascond ∪ tb ∪ fbfrom the branch annex domains, promotes the candidate branch's domain to it, intersects with the (complemented) condition, and prunes only if the kept branch spans all of them. Never readsnode.annex.domain, never reads.type. Sharesdomain_utils.concat_where_branch_domain()withinfer_domain._infer_concat_where, so the complement-before-promote ordering exists in one place.23b02ad50— aconcat_where.broadcast_branchespass making the implicit broadcast explicit, wired into both pass managers afterinfer_domain_opsand beforecanonicalize_domain_argument. After it every branch spans the full dimensions, so the branch-domain test is sufficient. It runstype_inference.reinferitself, which is legitimate at that point because it is before_infer_concat_wheredrops the type.The split is deliberate: commit 2 can be dropped, in which case the never-selected-branch bug stays open.
Both fixes are needed, they are not alternatives
The dimension-dropping bug is a hard compile failure, not a pessimisation —
inference.visit_SetAt'sassert expr_type.dims == target_type.dimsfires, reproduced from plain user code on gtfn. Underpython -Othe assert disappears and the mismatch reaches the backend. It requires statically known bounds, which is why the existing ffront tests miss it (casesproduces runtime bounds, so nothing prunes) and is exactly icon4py's compilation mode.The never-selected-branch bug is only a missed optimisation; the numbers are correct either way.
Cost, which is the least certain part
broadcast_branchesruns type inference and rewrites IR for everyconcat_wherewith a lower-dimensional branch — a common icon4py shape. Measured: gtfn output byte-identical for a non-prunable case (fuse_as_fieldopabsorbs thederef), the DaCe path produces exactly theas_fieldop(deref, D ∩ cond)its own lowering was constructing internally, and no test churn. But that is not evidence at scale, and it touches code currently being performance tuned.Testing
iterator_tests503 → 517. Every regression test verified to fail with its fix removed, keeping the module importable in each case. The ordering test asserts a value rather than pinning anAssertionError, so it also fails underpython -O. Fullintegration_testson roundtrip + numpy + gtfn: 2030 passed.ruff,tach,mypyclean.Follow-up worth noting
With
broadcast_branchesin the pipeline, the promotion path in_translate_concat_where_branch()(gtir_to_sdfg_concat_where.py) becomes unreachable, sincelen(source_expr.type.dims) < len(output_type.dims)can no longer hold. Left alone here — out of scope, andtranslate_concat_wherecould still be handed unnormalised IR.