feat[next-dace]: Use SDFG library node for lowering of broadcast and reduce by edopao · Pull Request #2386 · GridTools/gt4py

edopao · 2025-11-11T11:03:07Z

TODO:

Run ICON4Py CI, see ICON4Py PR#1240
Run Blueline

philip-paul-mueller

There are some refinements needed.

philip-paul-mueller · 2025-11-11T12:21:46Z

+
+
+@dace_library.node
+class Fill(dace_nodes.LibraryNode):


I would add some more semantic, i.e. an input connector, that collects the value that should be broadcasted and an output connector for the output.

I am also wondering if it would make sense to have two different library nodes.
One where the value that is broadcast is a literal, like 0.0 and one, which is probably the current one, where the value is read from another data descriptor (might be hard to integrate into the lowering).

philip-paul-mueller · 2025-11-12T05:53:39Z

@edopao
I am not sure if we should add the transformations we need already in this PR or in a later one.
If we put it in a later one, we should patch the optimizer to expand the node right at the beginning, this way we preserve the current behaviour and performance.

edopao · 2025-11-24T10:23:52Z

cscs-ci run default

edopao · 2025-11-24T14:43:47Z

cscs-ci run default

edopao · 2025-11-24T15:07:23Z

cscs-ci run default

edopao · 2025-12-10T07:29:43Z

No plan for now to integrate this feature.

…Did I even run the tests?

… confusing.

…o_sdfg_primitives.py Co-authored-by: Edoardo Paone <edoardo16@gmail.com>

edopao

Very good, just some minor comments.

edopao · 2026-05-06T08:05:24Z

+    library as dace_library,
+    nodes as dace_nodes,
+    properties as dace_properties,
+    subsets as dace_sbs,


In the transformation module, we use the dace_sbs alias, in the lowering module we use dace_subsets. It's OK to use dace_sbs in this module, but let's try to keep it consistent.

edopao · 2026-05-06T08:08:11Z

+    ```python
+    for i in range(len(broadcast_in_dim):
+        assert output.shape[broadcast_in_dim[i]] == value_to_broadcast.shape[i]
+    ```


Suggested change

```

```

In other words, the result array shape has the same size as the broadcast domain.

edopao · 2026-05-06T08:09:05Z

+    ```
+
+    Args:
+        broadcast_in_dim: How to broadcast.


Suggested change

broadcast_in_dim: How to broadcast.

broadcast_in_dim: How to broadcast, see the class documentation.

edopao · 2026-05-06T08:09:51Z

+
+    Args:
+        broadcast_in_dim: How to broadcast.
+        params: The parameters that should be used for the expansion. If given one


Suggested change

params: The parameters that should be used for the expansion. If given one

params: The parameters that should be used for the expansion. If given, one

edopao · 2026-05-06T08:10:25Z

+
+    Todo:
+        - While for the output it is probably okay to always require an adjacent
+            AccessNode for the input it might be possible to be on the other side


Suggested change

AccessNode for the input it might be possible to be on the other side

AccessNode, the input nodes might be outside a map scope.

However, I don't understand how this could happen.

edopao · 2026-05-06T12:17:07Z

+        # A fundamental requirement is that `bcast_result` is only generated by us.
+        #  ADR-18 guarantees us this if it is transient and has a single producer,
+        #  `bcast_node`. However, since we will remove `bcast_result`, we have to
+        #  make sure that it is not used every where else.


Suggested change

# make sure that it is not used every where else.

# make sure that it is not used anywhere else.

edopao · 2026-05-06T12:19:16Z

+
+            match consumer := consumer_edge.dst:
+                case dace_nodes.AccessNode():
+                    # TODO(phimuell): Are there more checks needed.


I suggest removing this todo comment before merge, unless there are known cases.

Suggested change

# TODO(phimuell): Are there more checks needed.

edopao · 2026-05-06T12:21:00Z

+        # Check single use data if it was not known at the beginning.
+        if self._single_use_data is None:
+            find_single_use_data = dace_analysis.FindSingleUseData()
+            single_use_data = find_single_use_data.apply_pass(sdfg, None)


Would it be wrong to now store single_use_data? I am asking because it is used again inside apply().

edopao · 2026-05-06T12:28:38Z

+        # We need new transformations in order to deal with GTIR library nodes.
+        # For now, we simply expand these nodes before starting optimizing.
+        # TODO: Remove once transformations are ready.


Why not calling ScalarBrodcastInliner before expanding?

edopao · 2026-05-06T12:29:43Z

+        #   probably yes, as we can remove the read and write of the initial data
+        #   only the write to final destination is left. If the consumers are Maps
+        #   the thing is a bit different. As we have to create the intermediate
+        #   allocation. If the read of the memory is okay the `InlineBroadcastAccess`


InlineBroadcastAccess does not exist yet.

edopao

Very good, just some minor comments.

edopao added 2 commits November 11, 2025 12:01

edit

f8180e2

edit

ef9ef92

edopao force-pushed the dace-fill_node branch from f94a037 to ef9ef92 Compare November 11, 2025 12:12

edopao requested a review from philip-paul-mueller November 11, 2025 12:15

undo extra change

bd1b766

philip-paul-mueller reviewed Nov 11, 2025

View reviewed changes

use library node also in concat_where

25abd36

edopao force-pushed the dace-fill_node branch from bc04cfc to 25abd36 Compare November 11, 2025 12:42

havogt reviewed Nov 11, 2025

View reviewed changes

Comment thread src/gt4py/next/program_processors/runners/dace/sdfg_library_nodes.py Outdated

edopao added 4 commits November 12, 2025 22:46

edit

071f512

Merge remote-tracking branch 'upstream/main' into dace-fill_node

331bcd3

edit

b90976b

fix for inf expressions

21a79d2

edopao commented Nov 13, 2025

View reviewed changes

Comment thread src/gt4py/next/program_processors/runners/dace/gtir_to_sdfg_primitives.py Outdated

edopao added 8 commits November 13, 2025 15:14

edit

a1f6f1a

edit

6e97232

Merge branch 'dace-refactor_concat_where' into dace-fill_node

08484df

edit

da12bdb

Merge branch 'dace-refactor_concat_where' into dace-fill_node

5382dce

Merge branch 'main' into dace-fill_node

b04586c

Merge branch 'main' into dace-fill_node

4647c7d

remove special handling for inf symbol

779b164

fix rebase

f250040

fix rebase

68a417f

Merge branch 'main' into dace-fill_node

9d774e4

philip-paul-mueller and others added 27 commits May 4, 2026 11:33

Made the inline expander accessible to the outside world.

6209739

Fixed some issues in the validatiobn function of the broadcast node. …

67d5ee4

…Did I even run the tests?

I do not fully understand why this is needed, sometimes NumPy is just…

755afea

… confusing.

Merge branch 'main' into dace-fill_node

b8f7d91

Fixed the expander, it should now work.

40eb027

Fixed the broadcasting test since unique_name() has been moved.

69cf288

Added a test for the library expansion, only scalar yet.

a31e163

Update src/gt4py/next/program_processors/runners/dace/lowering/gtir_t…

5249529

…o_sdfg_primitives.py Co-authored-by: Edoardo Paone <edoardo16@gmail.com>

Fixed an error in validation.

98e098a

Added a test to operate on vectors.

c7b1f2d

NOw we also do some slicing.

2444a2d

Discussion points with Edoardo.

c3cde1e

Fixed a wrong annotation.

b8d254e

Fixed some issue with renaming.

5b95cfa

Something is not working as expected.

c6c5bda

Refined how the MapToCopy detects broadcasts.

0cc7109

Handled how expansion of librarynodes is currently performed.

bd5b88e

Fixed extensive validation in FuseHorizontalConditionBlocks.

bec8ab8

Integrated the correction.

080bb32

Incorporated the domain correction at the expansion.

08b1d85

Made a unit test a bit harder.

44f524c

Makeing the optimnizer temporaraly more strict.

13541b0

Merge branch 'dace-fill_node_philip' into dace-fill_node

0645d58

Now also the parameter names are correctly set.

9592908

Implemented a missing case.

1e2089b

Forgot to update them.

8d3a89e

git Fixed a wrong check.

3dcf818

philip-paul-mueller mentioned this pull request May 6, 2026

DO NOT MERGE: Check Broadcast Node C2SM/icon4py#1240

Draft

edopao commented May 6, 2026

View reviewed changes

	```
	```
	In other words, the result array shape has the same size as the broadcast domain.

	broadcast_in_dim: How to broadcast.
	broadcast_in_dim: How to broadcast, see the class documentation.

	params: The parameters that should be used for the expansion. If given one
	params: The parameters that should be used for the expansion. If given, one

	AccessNode for the input it might be possible to be on the other side
	AccessNode, the input nodes might be outside a map scope.

	# make sure that it is not used every where else.
	# make sure that it is not used anywhere else.

Conversation

edopao commented Nov 11, 2025 • edited by philip-paul-mueller Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

philip-paul-mueller left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

philip-paul-mueller commented Nov 12, 2025

Uh oh!

Uh oh!

edopao commented Nov 24, 2025

Uh oh!

edopao commented Nov 24, 2025

Uh oh!

edopao commented Nov 24, 2025

Uh oh!

edopao commented Dec 10, 2025

Uh oh!

edopao 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!

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!

Choose a reason for hiding this comment

Uh oh!

edopao left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

edopao commented Nov 11, 2025 •

edited by philip-paul-mueller

Loading