Skip to content

feat[cartesian]: DaCe optimal for/map schedule#2628

Merged
romanc merged 5 commits into
GridTools:mainfrom
FlorianDeconinck:feat/dace_optimal_loop_schedule
Jul 3, 2026
Merged

feat[cartesian]: DaCe optimal for/map schedule#2628
romanc merged 5 commits into
GridTools:mainfrom
FlorianDeconinck:feat/dace_optimal_loop_schedule

Conversation

@FlorianDeconinck

Copy link
Copy Markdown
Contributor

Description

Reworked the based scheduling of the loops in OIR -> TreeIR to simplify the code and introduce an optimal scheduling for parallelism:

  • merge all map/for schedule code into a single _resolve_loop_schedule
  • respect syntax for sequential loops
  • default parallelism to maximize local parallelization (omp for parallel on CPU and plain kernel for CUDA)

This should be covered by the current unit tests

@FlorianDeconinck FlorianDeconinck requested review from romanc and twicki June 7, 2026 18:05
@romanc

romanc commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

I think the failing gpu-tests are due to a cuda-codgen issue in DaCe. From what I understand, this test case triggers the codepath here which inserts a global grid sync into a merged kernel. The grid sync insertion in successful, but the __gbar symbol isn't defined because it's not passed to the arguments of the nested SDFG. Similar, the (recreated) conditions inserted after the grid sync might (and in this case do) use undefined symbols that would need to be passed as arguments of the nested SDFG function. Both doesn't happen and this is why nvcc fails.

We could work around by slightly changing the test scenario.

@twicki twicki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks generally ok. @romanc and I quickly discussed that at some point the sequential concept that turns into loops should not have an (ignored) scheduleType anymore, but that can wait for a follow-up. Happy to merge this one as is

@romanc

romanc commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

looks like the new test needs to mark test running on gpu to only run on gpu, e.g.

pytest.param("dace:gpu", marks=[pytest.mark.requires_gpu])

@romanc

romanc commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

cscs-ci run default

@romanc romanc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

discussed that at some point the sequential concept that turns into loops should not have an (ignored) scheduleType anymore

  • Follow-up issue: #2681
  • PR to update the vertical loop representation in the treeir: #2688

@romanc

romanc commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

cscs-ci run default

@romanc romanc merged commit 20d1bea into GridTools:main Jul 3, 2026
20 checks passed
romanc added a commit to romanc/gt4py that referenced this pull request Jul 8, 2026
Follow-up from PR GridTools#2628 in which
we added a new test, but forgot to mark the dace-backends with
`pytest.mark.requires_dace`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants