dace icon indicating copy to clipboard operation
dace copied to clipboard

Fixed a bug in the map fusion transformation.

Open philip-paul-mueller opened this issue 1 year ago • 3 comments

Essentially the bug happens because the transformation considers any array with one elements as scalar, including arrays such as (1, 1). This commit allows the scalar branch of the transformation to handle this kind of situation. Another solution would be to "promote" it to an array.

In addition this commit adds some test cases for these kind of events.

philip-paul-mueller avatar Feb 26 '24 10:02 philip-paul-mueller

The test seems to pass on master. I think we should have a test that triggers the bug on master...

I looked a bit deeper into the transformation. I also think that it's better for the scalar branch to handle this case, because inside the map it's better to use a scalar transient instead of a transient array where all dimensions have size 1.

BenWeber42 avatar Feb 28 '24 17:02 BenWeber42

We briefly discussed this:

  • The scalar branch ist the better place to fix this (instead of array branch)
  • AlexNik confirmed that "0" is the correct subset for memlets from scalars

We are now looking into the assert (seems to highlight another bug) and whether the test triggers the bug (it triggers new code paths in the transformation, and, thus, increases code coverage, so is strictly an improvement).

BenWeber42 avatar Feb 29 '24 10:02 BenWeber42

As far as I can tell there is still a bug in the array branch.

philip-paul-mueller avatar Mar 14 '24 06:03 philip-paul-mueller