refactor[next-dace]: Add library node for reduce with skip values by edopao · Pull Request #2603 · GridTools/gt4py

edopao · 2026-05-15T10:54:12Z

DaCe has a library node in its standard library for reduction. However, GT4Py and ICON4Py also need to lower reduction expressions with skip values. The lowering was already done, on main, inside a nested SDFG. All this PR does is to refactor the code that creates the nested SDFG and make an application library node in GT4Py. In this way, the SDFG nodes for regular reduction and the nodes for reduction with skip values are represented in the same way in the SDFG.

No performance regression in ICON4Py blueline:

edopao · 2026-05-15T15:44:33Z

cscs-ci run dace

edopao · 2026-05-20T07:40:50Z

cscs-ci run default

## Description Do not apply `open_mp` flags on device compilers - we don't do multithreading host code for GPU backends (and device is plain wrong). This flag usage was discovered in the case of an `icx/nvcc` where the `-qopenmp` intel flag bled into the command line of `nvcc`. ## Requirements - [ ] All fixes and/or new features come with corresponding tests. - [ ] Important design decisions have been documented in the appropriate ADR inside the [docs/development/ADRs/](docs/development/ADRs/README.md) folder.

- Use same `dace` version for GT4Py cartesian and next. - Make `dace` a GT4Py dependency, and remove the development groups `dace-cartesian` and `dace-next`. - Instead of using the `gridtools` package index, pull the dace pre-release package from PyPI index. Note: if in the future a custom (non-pypi) version is needed, cartesian and next need to create a version from a common branch (and publish to the gridtools index).

This reverts commit 7d769d6.

philip-paul-mueller

There are some small things but nothing big.

philip-paul-mueller · 2026-05-27T05:01:18Z

+    wcr = dace_properties.LambdaProperty(default="lambda a, b: a")
+    identity = dace_properties.Property(default=0)
+    init = dace_properties.Property(default=0)


I think these properties should not have a default.

I tried, but it is not allowed:
Error importing plugin "gt4py.next.type_system.mypy_plugin": Default not properly defined for property [misc]

By the way, the dace standard reduce does the same.

Just a random question, did you specify a dtype value?
Looking at the error and Property::init() I would say that dtype is missing.
You can also play with allow_none.

The problem is that dtype is not known. Yes, I could play with allow_none indeed.

philip-paul-mueller · 2026-05-27T05:07:01Z

+        max_neighbors = mask_desc.shape[1]
+        assert isinstance(max_neighbors, int) or str(max_neighbors).isdigit()
+
+        local_dim_index = inedge.data.src_subset.size().index(max_neighbors)


Might break in cases where the number of {edges, cell, vertices} is the same as local dimensions.
Probably too paranoid and I do not have an alternative.
But you should probably put a TODO or NOTE here explaining what it is doing and what the problems are.

I have an idea. I can check that the size of the subset is 1 in all dimensions except one, the local dimension. I will push a new commit.

This should work (except for the case that there is only a single neighbour, which is probably unlikely).

philip-paul-mueller

Some small suggestions.

philip-paul-mueller · 2026-05-27T11:36:57Z

+    wcr = dace_properties.LambdaProperty(default="lambda a, b: a")
+    identity = dace_properties.Property(default=0)
+    init = dace_properties.Property(default=0)


Just a random question, did you specify a dtype value?
Looking at the error and Property::init() I would say that dtype is missing.
You can also play with allow_none.

philip-paul-mueller · 2026-05-27T11:49:21Z

+        max_neighbors = mask_desc.shape[1]
+        assert isinstance(max_neighbors, int) or str(max_neighbors).isdigit()
+
+        local_dim_index = inedge.data.src_subset.size().index(max_neighbors)


This should work (except for the case that there is only a single neighbour, which is probably unlikely).

philip-paul-mueller · 2026-05-27T12:01:03Z


+        # We now expand all GT4Py specific library nodes.
+        #  We do this such that we have control over all the Maps that are there.
+        # TODO(edopao,phimuell): It is probably the right place, but maybe there is a better one.


I am actually not sure if it is the best one.
I would maybe also think of moving it at the very end, however, given my experience with the broadcast node, we should probably keep it here for now.

philip-paul-mueller · 2026-05-27T12:01:29Z

        gtx_transformations.FuseHorizontalConditionBlocks(),
-        validate=True,
-        validate_all=True,
+        validate=False,


Good catch.

philip-paul-mueller

LGTM

edopao added 3 commits May 15, 2026 12:53

Add library node for reduce with skip values

e628361

avoid symbolic conversion of node properties (fix for sympy oo)

c466998

Move expansion of reduce node

308cfc7

edopao force-pushed the dace_reduce_skip_node branch from 4e1c0e5 to 308cfc7 Compare May 15, 2026 15:43

edopao force-pushed the dace_reduce_skip_node branch 2 times, most recently from 5fcb578 to bf658c3 Compare May 20, 2026 16:31

FlorianDeconinck and others added 8 commits May 21, 2026 10:06

edit

e89572d

edit

54a0c54

edit

ec731af

expand all reduce nodes before optimization

a9db106

do not expand reduce nodes

81bdc32

undo extra change

14443e2

edopao force-pushed the dace_reduce_skip_node branch from b8f0627 to 14443e2 Compare May 21, 2026 08:07

edopao added 8 commits May 21, 2026 14:07

expand reduce with skip values before opt

a8187b5

impl based on reduce node

7d769d6

fix

b47e948

Revert "impl based on reduce node"

01a05c7

This reverts commit 7d769d6.

Merge branch 'main' into dace_reduce_skip_node

f474149

undo extra change

243512e

undo extra change

5c1cd3b

expand library nodes as in Philip's PR

ef1bb8e

edopao requested a review from philip-paul-mueller May 26, 2026 15:07

edopao mentioned this pull request May 26, 2026

feat[next-dace]: Use SDFG library node for lowering of broadcast and reduce #2386

Draft

3 tasks

philip-paul-mueller reviewed May 27, 2026

View reviewed changes

edopao added 2 commits May 27, 2026 11:48

Turn assert into exception in validate()

4d15987

add check for local dimension

94a7595

edopao requested a review from philip-paul-mueller May 27, 2026 11:12

philip-paul-mueller reviewed May 27, 2026

View reviewed changes

address review comments

b9f8d92

edopao requested a review from philip-paul-mueller May 27, 2026 13:07

philip-paul-mueller approved these changes May 27, 2026

View reviewed changes

edopao merged commit 154bd05 into GridTools:main May 27, 2026
24 checks passed

edopao deleted the dace_reduce_skip_node branch May 27, 2026 13:29

Conversation

edopao commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

edopao commented May 15, 2026

Uh oh!

edopao commented May 20, 2026

Uh oh!

philip-paul-mueller left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

philip-paul-mueller left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

philip-paul-mueller left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

edopao commented May 15, 2026 •

edited

Loading