[f2dace/staging] `simplify()` simply breaks `velocity_tendendencies` SDFG.

[pratyai]

Verified on f2dace/staging branch (commit: 34e23611bedee4bed6c2d15e231e03e7faa5465c).

Before: https://polybox.ethz.ch/index.php/s/mD35Yj68CkwYJst
After: https://polybox.ethz.ch/index.php/s/yRaHt3MfNwKg3Jz

from dace import SDFG
from dace.transformation.interstate import LoopToMap
from dace.transformation.dataflow import MapCollapse

g = SDFG.from_file('~/Downloads/before.sdfgz')
g.simplify(skip=["ArrayElimination", "InlineSDFG"])

g.validate()
'''
output:
/Users/pmz/gitspace/dace/dace/sdfg/validation.py:551: UserWarning: WARNING: Use of uninitialized transient "cfl_clipping" in state "single_state_body_4"
  warnings.warn('WARNING: Use of uninitialized transient "%s" in state "%s"' %
/Users/pmz/gitspace/dace/dace/sdfg/validation.py:551: UserWarning: WARNING: Use of uninitialized transient "difcoef" in state "single_state_body"
  warnings.warn('WARNING: Use of uninitialized transient "%s" in state "%s"' %
/Users/pmz/gitspace/dace/dace/sdfg/validation.py:551: UserWarning: WARNING: Use of uninitialized transient "levmask" in state "single_state_body"
  warnings.warn('WARNING: Use of uninitialized transient "%s" in state "%s"' %
'''

g.view()
'''
Search on the UI to verify that there is a tasklet reading from `tmp_call_18` and assigning to somewhere.
'''

g.simplify(skip=["ArrayElimination", "InlineSDFG"])

g.view()
'''
Search on the UI to verify that `tmp_call_18` is nowhere to be found and that tasklet has been replaced by a constant.
'''

[ThrudPrimrose]

Update. This is an issue in LoopToMap transformation.
Here is the relevant part from the SDFG:

This is a a max-reduction. With a data dependency (in the if condition).
LoopToMap turns it into a map (wrong), see picture below.

This then gets further simplified, but it does not make too much sense why further steps break, when the current state is invalid. I assume, since this map has no output, DeadDataFlowElimination removes it (It removing this map is justified). Then the tmp_call_18 is replaced with its initial everywhere in ConstPropagation as the reduction is removed.

[ThrudPrimrose]

I can't view the output as we can't serialize SDFGs that have structs in main.
(I could flatten it with the option to remove all structs, then we should technically have a graph that might compile on main)

[ThrudPrimrose]

Chat log: Gives info on what is the issue. LoopToMap analysis part should be improved the detect loops that can become maps better.

Berke Ates
11:22

"fixed" your LoopToMap issue
it's basically permissive being permissive

just made permissive less permissive

Yakup Koray Budanaz
11:23
"permissive" more like "might explode"

Was there a reason we had to do it in permissive mode?

in the first place, or was it just something we did on a whime without thinking

Berke Ates
11:23
permissive ignores all write and read conflicts / dependencies
If you know that there are none, you can "force" LoopToMap to apply

Yakup Koray Budanaz
11:23
I see

Sounds like we should try to not have this flag in the pipeline

I thought it was more of something like

"i am unsure, but I will do it"

but not:
"I know this is wrong but I will do it regardles..."

Berke Ates
11:25
iirc we did have cases where we knew that it can be parallelized
but LoopToMap was not certain (due to the way thread locality analysis works) and did not apply

so we added bullying permissive

To convince LoopToMap that it's safe

But that was after the reductions
If we don't add reductions, turning reducing loops into maps is ofc wrong

Yakup Koray Budanaz
11:36
I see, the mystery is solved

To save it, I will copy paste the messages to the github issue, would that be ok?

Such that we dont return to this issue

New Messages

Berke Ates
11:36
no problem

But I did change permissive to not ignore symbols though
So it should not apply to the last loop anymore

Yakup Koray Budanaz
11:36
It looks like analysis needs to be improved later