stree -> sdfg: add missing nested SDFG connectors - #2548
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
774fa14 to
72d64b2
Compare
|
/cc @FlorianDeconinck this is ready for review (as discussed). Can't ping you as reviewer because I don't have rights to do so ... |
This comment was marked as outdated.
This comment was marked as outdated.
72d64b2 to
1e32fb3
Compare
|
I don't understand how this can/should break ML tests. I'm hitting a 404 again. Seems like something isn't online (anymore)?! |
Downloading models is failing, cloudflare must be down or something |
ThrudPrimrose
left a comment
ThrudPrimrose
left a comment
There was a problem hiding this comment.
Core fix looks good. Semantically, might be better to split the bugfix and syntactic-cleaning, regardless all good.
|
Need to wait until cloudflare or whatever webservice that is down is back |
I can split stuff if you want me to. Since we anyway have to wait and everything is neatly in commits it's gonna be very easy and we won't loose time. |
Do these models change often? If not, could we cache them or are they way too big for GHA caches? |
|
1e32fb3 to
00c6121
Compare
|
I've split out the unrelated changes. Someone with access rights will need to kick off the GPU tests ... |
|
cscs-ci run GPU |
3 participants
Description
Follow-up from PR #2541: there's one more case of "read after write" where we need to add a connector to the nested SDFG to retain the connection to "outside" memory and not confuse simplify.
Founds as part of our work to port the FV3 dynamical core.
This is DelnFlux_2, last state. Look at
__g_self__fy2: it's read after written and thus no input connector for__g_self__fy2is added. That's not good because__g_self__fy2is only partially written here. Also, since there's no input connector and since the output connector will be (rightfully) pruned,__g_self__fy2will be "inlined" byDeadDataflowElimination, which then breaks stuff down the line (e.g. latest inD_SW).The PR adds a simpler reproducer (in tests) and ensures as part of the unit tests that this won't regress.