StateFusion misses read-write conflict due to early return
#1954
Conversation
This file contains 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
… `memlets_intersect`
|
This looks reasonable, makes sense. I would add a test though with the SDFG API and a certain node addition ordering for sanity’s sake. |
|
Unit tests in. Instead of figuring out a test that would pass most check but the one we are fixing, I am testing that one check ( |
2 tasks
tbennun
approved these changes
Mar 5, 2025
FlorianDeconinck
added a commit
to FlorianDeconinck/dace
that referenced
this pull request
Mar 5, 2025
Check for all potential match in `_check_paths` to not miss potential `memlets_intersect` failure. Unit tests for checks the internal `_check_paths` function since it triggers on a non deterministic networkx search. :warning: This code exists as-is in `main` as of Feb 26 and showcase the same issue, cherry-picking fix will have to happen :warning:
tbennun
pushed a commit
that referenced
this pull request
Mar 6, 2025
Bring a V1 maintenace fix PR'ed here: #1954 Original commit messages: - Check for all potential matches in `_check_paths` to not miss potential `memlets_intersect` failure. - Unit test checks the internal `_check_paths` function since it triggers on a non deterministic networkx search.
romanc
added a commit
to GridTools/gt4py
that referenced
this pull request
Mar 18, 2025
## Description This PR refactors the GT4Py/DaCe bridge to expose control flow elements (`if` statements and `while` loops) to DaCe. Previously, the whole contents of a vertical loop was put in one big Tasklet. With this PR, that Tasklet is broken apart in case control flow is found such that control flow is visible in the SDFG. This allows DaCe to better analyze code and will be crucial in future (within the current milestone) performance optimization work. The main ideas in this PR are the following 1. Introduce `oir.CodeBlock` to recursively break down `oir.HorizontalExecution`s into smaller pieces that are either code flow or evaluated in (smaller) Tasklets. 2. Introduce `dcir.Condition`and `dcir.WhileLoop` to represent if statements and while loops that are translated into SDFG states. We keep the current `dcir.MaskStmt` / `dcir.While` for if statements / while loops inside horizontal regions, which aren't yet exposed to DaCe (see #1900). 3. Add support for `if` statements and `while` loops in the state machine of `sdfg_builder.py` 4. We are breaking up vertical loops inside stencils in multiple Tasklets. It might thus happen that we write a "local" scalar in one Tasklet and read it in another Tasklet (downstream). We thus create output connectors for all scalar writes in a Tasklet and input connectors for all reads (unless previously written in the same Tasklet). 5. Memlets can't be generated per horizontal execution anymore and need to be more fine grained. `TaskletAccessInfoCollector` does this work for us, duplicating some logic in `AccessInfoCollector`. A refactor task has been logged to fix/re-evaluate this later. This PR depends on the following (downstream) DaCe fixes - spcl/dace#1954 - spcl/dace#1955 which have been merged by now. Follow-up issues - unrelated changes have been moved to #1895 - #1896 - #1898 - #1900 Related issue: GEOS-ESM/NDSL#53 ## Requirements - [x] All fixes and/or new features come with corresponding tests. Added new tests and increased coverage of horizontal regions with PRs #1807 and #1851. - [ ] Important design decisions have been documented in the appropriate ADR inside the [docs/development/ADRs/](docs/development/ADRs/Index.md) folder. Docs are [in our knowledge base](https://geos-esm.github.io/SMT-Nebulae/technical/backend/dace-bridge/) for now. Will be ported. --------- Co-authored-by: Roman Cattaneo <> Co-authored-by: Roman Cattaneo <1116746+romanc@users.noreply.github.com>
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.
Check for all potential match in
_check_pathsto not miss potentialmemlets_intersectfailure.Unit tests for checks the internal
_check_pathsfunction since it triggers on a non deterministic networkx search.mainas of Feb 26 and showcase the same issue, cherry-picking fix will have to happen