fix[next]: concat_where(Dim != value, ...) gives wrong results - #2819
Merged
tehrengruber merged 3 commits intoAug 25, 2026
Conversation
`InferDomainOps` rewrote `IDim != a` into `IDim < a & IDim > a`. The two half-open ranges are disjoint, so the intersection is empty and the true branch of a `concat_where` with such a condition could never be selected: `concat_where(IDim != a, x, y)` silently evaluated to `y` everywhere. Lower to `IDim < a | IDim > a` instead. `canonicalize_domain_argument` already expands the union into a nested `concat_where`, which is the same form it produces for the equivalent `IDim == a` condition with swapped branches. Only the compiled path was affected; in embedded execution `Dimension.__ne__` with an integer raises `NotImplementedError` (ADR 22). The existing unit test asserted the `and_` shape and hence pinned the bug, and no integration test used `!=` at all. Adds integration tests for the operand forms that had no coverage: `Dim != n`, `Dim <= n`, and the reversed `n > Dim`, `n >= Dim`, `n == Dim`, `n != Dim`. Only `!=` was broken; the others were correct but untested. Completes the unit test table with the operand orders that were missing for `eq` / `not_eq`.
The tests that allocate all arguments and the output on the default domain repeat the allocation and `cases.verify` boilerplate that `cases.verify_with_default_data` already encapsulates: it allocates the default data, applies `asnumpy` and calls the reference with the arguments. Converts the nine such tests. The remaining ones keep the explicit form since they either restrict a domain (`allocate(..., domain=)` / `sizes=`) or fix the value of a scalar argument, neither of which the default allocation can express.
tehrengruber
force-pushed
the
fix_not_eq_domain_inference
branch
from
August 24, 2026 15:52
dfa939a to
c042178
Compare
tehrengruber
changed the title
Dim != value domain condition to a unionconcat_where(Dim != value, ...) gives wrong results
havogt
approved these changes
Aug 25, 2026
There was a problem hiding this comment.
Pull request overview
Fixes compiled concat_where(Dim != value, ...) by lowering inequality to a union of less-than and greater-than domains.
Changes:
- Corrects
!=domain lowering. - Updates unit expectations.
- Adds integration and comparison coverage.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Summary |
|---|---|
tests/next_tests/unit_tests/iterator_tests/transforms_tests/test_infer_domain_ops.py |
Verifies union-based lowering. |
tests/next_tests/integration_tests/feature_tests/ffront_tests/test_concat_where.py |
Adds regression and comparison coverage. |
src/gt4py/next/iterator/transforms/infer_domain_ops.py |
Corrects != domain transformation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Noticed while reviewing #2795, see https://github.com/GridTools/gt4py/pull/2795\#issuecomment-5395705263. Independent of that PR — the bug predates it and reproduces identically on
main.Problem
InferDomainOpsrewroteIDim != aintoIDim < a & IDim > a. The two half-open ranges are disjoint, so the intersection is empty and the true branch of aconcat_wherewith such a condition can never be selected:concat_where(IDim != a, x, y)silently evaluates toyeverywhere.With
a = ones,b = zerosonI: [0, 6):Only the compiled path is affected; in embedded execution
Dimension.__ne__with an integer raisesNotImplementedError(ADR 22).Fix
Lower to
IDim < a | IDim > a.canonicalize_domain_argumentalready expands the union into a nestedconcat_where— it is the same form it produces for the equivalentIDim == acondition with swapped branches, so no new machinery is involved.Why this was not caught
test_infer_domain_ops.pyasserted theand_shape: the expected value was transcribed from the implementation rather than derived from the semantics, so the test pinned the bug instead of catching it.!=at all. The construct reads as unsupported per ADR 22, but that is only enforced in embedded execution, where the field operator body runs as Python — a traced field operator never callsDimension.__ne__.Tests
For the bug itself: the unit test that pinned the
and_shape now asserts the union, and an integration test forconcat_where(Dim != n, ...)is added.Independently of the bug, integration tests were added for the remaining comparison operator and operand-order combinations that had no coverage, and the existing
concat_wheretests were simplified.AI disclaimer: This PR was written with the help of AI tools. I reviewed the fix in detail and the respective unit tests in detail. The test refactoring was a simple precise prompt, should be alright, but I only browsed through the changes.