Transformation Bugs (Auto Optimize)

[philip-paul-mueller]

The new version of MapFusion is able to handle more cases and generates slightly different graphs. Some other transformations mostly related to auto optimize do not handle these cases properly and fail.
Either the generate wrong graphs (validation fails), but perform some operations which are invalid.
To pass the CI the new MapFusion was equipped with a compatibility flag strict_dataflow which generated graphs, that were more in line with what other transformations expect.

This issues is created to collect all known transformations or cases that cause an error.
Since mostly all of them are related to the auto optimizer, you should apply the following patch:

From d7091136eb6c0110f77795e8cfe3743b9b816c60 Mon Sep 17 00:00:00 2001
From: Philip Mueller <philip.mueller@cscs.ch>
Date: Thu, 5 Sep 2024 11:31:50 +0200
Subject: [PATCH] Activate the new MapFusion behaviour.

---
 dace/transformation/auto/auto_optimize.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/dace/transformation/auto/auto_optimize.py b/dace/transformation/auto/auto_optimize.py
index 09ff481e3..69d3f0a68 100644
--- a/dace/transformation/auto/auto_optimize.py
+++ b/dace/transformation/auto/auto_optimize.py
@@ -52,6 +52,9 @@ def greedy_fuse(graph_or_subgraph: GraphViewType,
     :param permutations_only: Disallow splitting of maps during MultiExpansion stage
     :param expand_reductions: Expand all reduce nodes before fusion
     """
+    validate=True
+    validate_all=True
+    strict_dataflow=False
     debugprint = config.Config.get_bool('debugprint')
     if isinstance(graph_or_subgraph, ControlFlowRegion):
         if isinstance(graph_or_subgraph, SDFG):
@@ -61,7 +64,7 @@ def greedy_fuse(graph_or_subgraph: GraphViewType,
             #  We have to use `strict_dataflow` because it is known that `CompositeFusion`
             #  has problems otherwise.
             graph_or_subgraph.apply_transformations_repeated(
-                    MapFusion(strict_dataflow=True),
+                    MapFusion(strict_dataflow=strict_dataflow),
                     validate_all=validate_all,
             )
 
@@ -82,7 +85,7 @@ def greedy_fuse(graph_or_subgraph: GraphViewType,
         if isinstance(graph_or_subgraph, SDFGState):
             sdfg = graph_or_subgraph.parent
             sdfg.apply_transformations_repeated(
-                    MapFusion(strict_dataflow=True),
+                    MapFusion(strict_dataflow=strict_dataflow),
                     validate_all=validate_all,
             )
             graph = graph_or_subgraph
-- 
2.46.0

The commit b2dea1d1 in Philip's is known to pass the test suite.

The compatibility mode is by default enabled. However, with this patch you can enable it (you have to look also at auto optimize).

From 6ffd03918647c725b60377c1b00bad9263deb6cd Mon Sep 17 00:00:00 2001
From: Philip Mueller <philip.mueller@cscs.ch>
Date: Thu, 5 Sep 2024 11:40:37 +0200
Subject: [PATCH] The compability mode is now disabled.

---
 dace/transformation/dataflow/map_fusion_helper.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/dace/transformation/dataflow/map_fusion_helper.py b/dace/transformation/dataflow/map_fusion_helper.py
index d52e8d4a9..59a4b4040 100644
--- a/dace/transformation/dataflow/map_fusion_helper.py
+++ b/dace/transformation/dataflow/map_fusion_helper.py
@@ -21,7 +21,7 @@ class MapFusionHelper(transformation.SingleStateTransformation):
     Args:
         only_inner_maps: Only match Maps that are internal, i.e. inside another Map.
         only_toplevel_maps: Only consider Maps that are at the top.
-        strict_dataflow: If `True`, the default, the transformation ensures a more
+        strict_dataflow: If `True`, the transformation ensures a more
             stricter version of the data flow.
 
     Note:
@@ -56,7 +56,7 @@ class MapFusionHelper(transformation.SingleStateTransformation):
     )
     strict_dataflow = properties.Property(
         dtype=bool,
-        default=True,
+        default=False,
         desc="If `True` then the transformation will ensure a more stricter data flow.",
     )
 
-- 
2.46.0

[philip-paul-mueller]

Correlation Test

Affects tests/npbench/polybench/correlation_test.py, the main issue is the dynamic Memlet and the multiple overwriting of data.
In the non strict mode map fusion will generate the following SDFG:

In the top map X (stddev) is written and it is written back, the access node does not have an output degree.
In the second map the data is modified again, it is important that the lower map uses a dynamic memlet to write the data back, so actually X should also be seen as an input of the map, but it is not.

This is the result after MapFusion has run, in non strict mode. The SDFG is still correct, because the top map is sequenced before the lower map, since they are connected (through the unnamed access node).

Auto optimize, I think it is comonent fusion, will now merge these two maps together. However, it does not detect the data dependency in stddev thus the computation of the top map are lost.

[philip-paul-mueller]

I had to heavily rework SDFGState._read_and_write_sets() since it has caused some problems with subsets that were not set. Essentially, I have rewritten the function and fixed some odd behaviour.

As a side effect the test transformations/move_loop_into_map_test.py::MoveLoopIntoMapTest::test_apply_multiple_times failed, It was now possible to apply a transformation that was not possible before. I looked at it very carefully and I am sure that it is legal to do it, also the comment of the test was "probably to conflicts". I modified the test such that it can be applied now and also modified a test to ensure that the same computations happens before and after the transformation.

See also 567f459eea5adf3ee0d3c803880b2e1e4874b097 and 9cad08dae2a0d7a2680a2bb8fa219a10f594b26c.

[philip-paul-mueller]

If the auto optimizer does not use the strict data flow mode, then useless size 1 dimensions of intermediate are removed. In that case tests/npbench/weather_stencils/vadv_test.py will lead an error.

The error is generated in dace/transformation/dataflow/redundant_array.py:1000 where two subsets are subtracted, however, these subsets have different dimensionality.
I was hopping that PR#1637 would solve it, but not.

[philip-paul-mueller]

This issue was now split into separate ones.
See PR#1637 for more.