Reduce JIT-heavy structural test coverage in transformation tests#659
Open
Reduce JIT-heavy structural test coverage in transformation tests#659
Conversation
Reduce the JIT burden in transform loop tests by validating transformed loop structure directly in the IR. Keep a small runtime smoke suite for the subtle fusion, fission and unroll interactions that still need end-to-end coverage.
Reduce runtime compilation in transform region tests by checking hoisted assignment and loop structure directly in the IR. Keep the pragma-inlining and promotion cases as end-to-end smoke tests because they still provide valuable semantic coverage.
Rename the local assignment helper and compare Loki assignment nodes directly instead of stringifying lhs and rhs expressions. This keeps the structural assertions aligned with Loki's native symbol and expression comparison support.
Rename the local assignment helper in transform loop tests and compare Loki assignment nodes directly instead of stringifying expressions. This keeps the IR assertions aligned with Loki's native symbol and expression comparison support.
Rename the local loop helpers to reflect native Loki node comparisons and stop stringifying loop variables and literal bounds in the transform loop and region tests. This keeps the structural IR assertions aligned with Loki's native comparison behavior for symbols, expressions and numeric literals.
Use native Loki comparisons for conditional expressions and variable shapes in the transform loop tests instead of stringifying those IR nodes. This finishes the recent cleanup toward structural assertions that rely on Loki's native comparison support.
Reduce runtime compilation in outline transformation tests by asserting outlined routines, calls and inferred arguments directly in the IR. Keep the imports, derived-type and associate cases as end-to-end smoke tests because they still provide valuable semantic coverage across module boundaries and richer argument handling.
Reduce runtime compilation in the loop blocking tests by checking split loop structure, generated helper variables and blocked array accesses directly in the IR. Keep the multi-intent and higher-dimensional blocking cases as end-to-end smoke tests because they still provide useful semantic coverage.
|
Documentation for this branch can be viewed at https://sites.ecmwf.int/docs/loki/659/index.html |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #659 +/- ##
==========================================
- Coverage 96.39% 96.35% -0.04%
==========================================
Files 266 266
Lines 46418 45925 -493
==========================================
- Hits 44745 44252 -493
Misses 1673 1673
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Remove redundant imports and drop unused parametrization from the transformed IR-only test cases. This keeps the structural tests lint-clean without carrying runtime-oriented parameters that no longer affect coverage.
awnawab
requested changes
Mar 31, 2026
|
|
||
| c = a + b | ||
| end subroutine test_outline | ||
| end subroutine test_outline |
Contributor
There was a problem hiding this comment.
optional: the subroutine closing statement should have the same indent as the opening
| a(i) = real(i, kind=8) | ||
| end do | ||
| end subroutine test_1d_splitting | ||
| end subroutine test_1d_splitting |
Contributor
There was a problem hiding this comment.
optional: same comment about end statement indentation
| assert np.array_equal(a, a_ref), "a should be equal to a_ref=(1, 2, ..., n)" | ||
|
|
||
| clean_test(filepath) | ||
| assert loop_symbols(routine.ir) == [ |
Contributor
There was a problem hiding this comment.
It would be nice to add a test that verifies if the value of the inner loop upper bound is indeed block_size
| assert np.array_equal(a, a_ref), "a should be equal to a_ref=(1, 2, ..., n)" | ||
|
|
||
| clean_test(filepath) | ||
| assert loop_symbols(routine.ir) == [ |
Contributor
There was a problem hiding this comment.
Same comment about block_size
| assert np.array_equal(b, b_ref), "b should equal b_ref" | ||
|
|
||
| clean_test(filepath) | ||
| assert loop_symbols(routine.ir) == [ |
| assert np.array_equal(a, a_ref), "a should be equal to a_ref=(1, 2, ..., n)" | ||
|
|
||
| clean_test(filepath) | ||
| assert loop_symbols(routine.ir) == [ |
| end do | ||
| end do | ||
| end subroutine transform_loop_interchange | ||
| end subroutine transform_loop_interchange |
Contributor
There was a problem hiding this comment.
optional: mismatched end statement indentation
| end do | ||
| end do | ||
| end subroutine transform_loop_interchange_project | ||
| end subroutine transform_loop_interchange_project |
Contributor
|
Thanks a lot @mlange05 for this cleanup 🙏 and congrats on your first agentic PR 😉 I've left a couple of minor comments on where I think the test coverage should be expanded, but apart from that this is essentially G2G 👌 |
Collaborator
Author
|
@awnawab Well spotted. Should all be addressed now. |
awnawab
approved these changes
Apr 1, 2026
Contributor
awnawab
left a comment
awnawab
left a comment
There was a problem hiding this comment.
Thanks a lot for the prompt fixes, this is now G2G 👌
mlange05
added
the
ready to merge
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
2 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.
Summary
Details
This change reduces the amount of runtime compilation used in tests whose main purpose is to validate transformation structure rather than compiled behavior.
The refactor focuses on:
loki/transformations/tests/test_transform_loop.pyloki/transformations/tests/test_transform_region.pyloki/transformations/extract/tests/test_outline.pyloki/transformations/tests/test_loop_blocking.pyAcross these files, simple end-to-end JIT checks are replaced with direct assertions on:
The remaining JIT tests act as smoke coverage for cases that still benefit from runtime validation, such as:
The branch also updates the local testing style to prefer Loki's native comparison semantics over stringification in structural assertions, and documents that guidance in
AGENTS.md.Validation
pytest loki/transformations/tests/test_transform_loop.pypytest loki/transformations/tests/test_transform_region.pypytest loki/transformations/extract/tests/test_outline.pypytest loki/transformations/tests/test_loop_blocking.py