fix[next-dace]: reject in-map global write racing with an independent read - #2815
Open
edopao wants to merge 2 commits into
Open
fix[next-dace]: reject in-map global write racing with an independent read#2815edopao wants to merge 2 commits into
edopao wants to merge 2 commits into
Conversation
… read GT4PyMapBufferElimination inlines the write-back 'MapExit -> tmp -> G' into the Map body. When the Map also reads 'G' through a branch that is independent of the branch producing the written value, the rewrite creates a write-after-read hazard: independent branches inside a Map scope can be scheduled in arbitrary (insertion) order, so the read may observe the newly written value and produce silently wrong results. Reject the rewrite unless every read of 'G' inside the Map is dataflow-ordered before the branch that produces the written value. Reads that feed the producing branch (in-place patterns such as 'G[i] = f(G[i])') remain admissible. Observed as wrong numerical results in icon4py stencil tests with the dace backend (e.g. 'extrapolate_temporally_exner_pressure'). The added unit test fails without this fix because the transformation is applied despite the hazard.
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.
Description
GT4PyMapBufferEliminationinlines the write-backMapExit -> tmp -> Ginto the Map body, eliminating the transient buffer. When the Map also readsGthrough a branch that is independent of the branch producing the written value, the rewrite creates a write-after-read hazard: independent branches inside a Map scope can be scheduled in arbitrary (insertion) order, so the read may observe the newly written value instead of the old one, producing silently wrong results.This showed up as wrong numerical output in the icon4py stencil tests with the dace backend (e.g.
extrapolate_temporally_exner_pressure): a stencil computing an auxiliary field from a field it also updates in place read the already-overwritten value.The fix adds a check to
_perform_pointwise_test: the rewrite is rejected unless every read ofGinside the Map is dataflow-ordered before the branch that produces the written value (new helper_map_reads_glob_before_write). Reads that feed the producing branch — in-place patterns of the formG[i] = f(G[i])produced by a single branch — are ordered before the write by construction and remain admissible, so existing valid uses are unaffected.A unit test reproducing the hazard pattern is added: without this fix the transformation is applied despite the hazard (test fails), and execution of the rewritten SDFG yields numerically wrong results.
Requirements