-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix[dace][next]: Update MapFusion #1857
Merged
philip-paul-mueller
merged 12 commits into
GridTools:main
from
philip-paul-mueller:updated_map_fusion
Feb 14, 2025
Merged
fix[dace][next]: Update MapFusion #1857
philip-paul-mueller
merged 12 commits into
GridTools:main
from
philip-paul-mueller:updated_map_fusion
Feb 14, 2025
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
… the PR into GT4Py. The reason is that it is no longer economic to backport stuff we have to start fresh. However, we will now do some development on it here.
This file is essentially how the final map fusion in DaCe should look like. It is fully backwards compable, in the sense that parallel map fusion has to be enabled. We will now start to integrate it into our toolchain.
However, some of them will be removed later.
They are only very small this does not produce a lot of code.
edopao
approved these changes
Feb 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
src/gt4py/next/program_processors/runners/dace/transformations/auto_optimize.py
Outdated
Show resolved
Hide resolved
src/gt4py/next/program_processors/runners/dace/transformations/map_fusion.py
Outdated
Show resolved
Hide resolved
src/gt4py/next/program_processors/runners/dace/transformations/map_fusion.py
Outdated
Show resolved
Hide resolved
src/gt4py/next/program_processors/runners/dace/transformations/map_fusion.py
Outdated
Show resolved
Hide resolved
src/gt4py/next/program_processors/runners/dace/transformations/map_fusion.py
Outdated
Show resolved
Hide resolved
src/gt4py/next/program_processors/runners/dace/transformations/map_fusion.py
Outdated
Show resolved
Hide resolved
src/gt4py/next/program_processors/runners/dace/transformations/map_fusion.py
Outdated
Show resolved
Hide resolved
Co-authored-by: edopao <[email protected]>
Essentially recreates [`b38ba0b`](spcl/dace@b38ba0b), which fixes non deterministic behaviour in the remapping function.
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.
Last year, the then state of MapFusion from DaCe PR#1629 was added to GT4Py, as a temporary fix until the PR in DaCe is merged and parallel map fusion has become available there.
However, during that time the transformation in the PR has evolved and improved and some of the bug that were fixed are now appearing in GT4Py, for example PR#1850 and PR#1856.
Thus this PR updated the MapFusion transformation that is currently inside GT4Py and replaces it with newest development version from DaCe.
Because we need it, and it was designed from the start to be that way, it also adds parallel map fusion to the transformation.
As before, this transformation, currently fully located in
map_fusion_dace.py
, is only kept inside the repo until DaCe has caught up to it.The PR also introduces some additional memory layer that encapsulates the DaCe transformation.
Something that we have to deal with in the long run and we currently do because other parts of the toolchain require it.