Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix[next][dace]: fix map fusion and loop blocking #1856

Merged
merged 395 commits into from
Feb 11, 2025
Merged

fix[next][dace]: fix map fusion and loop blocking #1856

merged 395 commits into from
Feb 11, 2025

Conversation

edopao
Copy link
Contributor

@edopao edopao commented Feb 10, 2025

Improve optimization for icon4py stencil apply_diffusion_to_vn by means of two changes:

  • Ignore check of dynamic volume property on memlet that prevented serial map fusion.
  • Add support for NestedSDFG nodes in loop blocking transformation.

edopao and others added 30 commits January 9, 2025 12:55
@edopao
remove support for tuple iterator
2c1156b
@edopao
fix test marker
ac24404
@edopao
move 2 nested function definitions to separate helper functions
a414eba
@edopao
import changes from dace-gtir-iterator_view
311c74e
@edopao
edit test markers
015f69c
@edopao
edit test markers
f05a730
@edopao
Revert "edit test markers"
2363b62 
This reverts commit f05a730.
@tehrengruber
Fix failing tests
61feb99
@tehrengruber
Merge remote-tracking branch 'origin_tehrengruber/field_arg_with_non_…
30a4911 
…zero_domain_start' into field_arg_with_non_zero_domain_start
@tehrengruber
Merge branch 'main' into field_arg_with_non_zero_domain_start
052c54b
@edopao
Merge remote-tracking branch 'origin/main' into dace-gtir-iterator_view
fd1462d
@edopao
Merge remote-tracking branch 'origin/dace-gtir-iterator_view' into da…
8deaa16 
…ce-gtir-scan
@edopao
fix merge conflicts
9eb0c20
@edopao
fix merge conflicts (1)
90a2378
@edopao
remove wrong assert
db94493
@edopao
Merge remote-tracking branch 'origin/dace-gtir-iterator_view' into da…
76168b3 
…ce-gtir-scan
@tehrengruber
Address review comments
0d903cc
@tehrengruber
Merge remote-tracking branch 'origin_tehrengruber/field_arg_with_non_…
7b77c9f 
…zero_domain_start' into field_arg_with_non_zero_domain_start
@tehrengruber
Merge origin/main
a6cf988
@edopao
edit code comments
f7b18b3
@edopao
add tuple_get
87b5bd5
@edopao
Merge remote-tracking branch 'origin/dace-gtir-iterator_view' into da…
4914de5 
…ce-gtir-scan
@edopao
better symbol mapping for scan nested SDFG
415e251
@edopao
better symbol mapping for lambda nested SDFG
d93a387
@edopao
Merge branch 'dace-gtir-scan' into dace-gtir
9cacbb8
@edopao
Merge remote-tracking branch 'DropD/gtir-sdfg-convertible' into dace-…
2bb102a 
…gtir
@edopao
Fix attribute dtype error
3afb1cb
@edopao
Fix attribute dtype error
9c884e3
@edopao
Merge remote-tracking branch 'DropD/gtir-sdfg-convertible' into dace-…
a002d35 
…gtir
@edopao
fix mypy error
3ecaab4
edopao and others added 19 commits February 4, 2025 14:55
@edopao
Merge remote-tracking branch 'origin/main' into dace-main
39a3049
@edopao
Merge remote-tracking branch 'origin/dace-main' into gtir-dace
48c4dbd
@edopao
update uv lock
8774462
@edopao
Merge remote-tracking branch 'origin/dace-main' into gtir-dace
d068dbf
@edopao
make if_ always with exclusive branch execution
7b953db
@edopao
Merge remote-tracking branch 'origin/gtir-dace-exclusive_if' into gti…
3466be6 
…r-dace
@edopao
allow if-tasklets
ff37cd8
@edopao
enable deref of iterators outside of conditional blocks
5246b79
@edopao
Merge remote-tracking branch 'origin/main' into gtir-dace-exclusive_if
c6ae8b1
@edopao
Revert "allow if-tasklets"
715b8a2 
This reverts commit ff37cd8.
@edopao
Merge remote-tracking branch 'origin/gtir-dace-exclusive_if' into gti…
52d9287 
…r-dace
@edopao
remove extra-change
5a31efe
@edopao
Merge remote-tracking branch 'origin/main' into gtir-dace-exclusive_if
cf4b465
@philip-paul-mueller
Fixed the map fusion.
6095829
@edopao
review comments
02382fe
@edopao
Merge remote-tracking branch 'origin/gtir-dace-exclusive_if' into gti…
474e53f 
…r-dace
@edopao
Merge remote-tracking branch 'philip/dace-gtir-fixed-map-fusion' into…
90638f0 
… gtir-dace
@edopao
Merge remote-tracking branch 'origin/main' into gtir-dace
4bfa5b0
@edopao
fix map fusion and loop blocking for apply_diffusion_to_vn
3626e17
philip-paul-mueller
philip-paul-mueller reviewed Feb 11, 2025
View reviewed changes
Copy link
Contributor

@philip-paul-mueller philip-paul-mueller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some issues, but it looks generally good.

src/gt4py/next/program_processors/runners/dace/transformations/map_fusion_serial.py Show resolved Hide resolved
src/gt4py/next/program_processors/runners/dace/transformations/loop_blocking.py Show resolved Hide resolved
philip-paul-mueller
philip-paul-mueller approved these changes Feb 11, 2025
View reviewed changes
Copy link
Contributor

@philip-paul-mueller philip-paul-mueller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

There are some small things:

  • Please move the check of the symbol_mapping of the NestedSDFG to the other NestedSDFG specific cases, see the comment for more detail.
  • In test I would suggest the default: case of the match to throw a NotImplementedError instead of an assert, I think it is a bit more specific. But this is probably a matter of taste.
  • I have fully ignored the changes to map_fusion_serial.py as it will be superseded by fix[dace][next]: Update MapFusion #1857

src/gt4py/next/program_processors/runners/dace/transformations/loop_blocking.py Outdated Show resolved Hide resolved
src/gt4py/next/program_processors/runners/dace/transformations/map_fusion_serial.py Show resolved Hide resolved
.../program_processor_tests/runners_tests/dace_tests/transformation_tests/test_loop_blocking.py Outdated Show resolved Hide resolved
@edopao edopao merged commit 64b90dc into GridTools:main Feb 11, 2025
23 checks passed
@edopao edopao deleted the gtir-dace branch February 11, 2025 13:14
@philip-paul-mueller philip-paul-mueller mentioned this pull request Feb 12, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philip-paul-mueller philip-paul-mueller philip-paul-mueller approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@edopao @philip-paul-mueller @tehrengruber