[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass

soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-587124344 @zhiics I merged your changes and updated the branch. Would you mind taking another look?

[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass

soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586541787 It seems like we need to flip the args. I’ll open the issue on Monday.

[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass

soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586537875 Alright, sorry for all of the spam. All tests are now fixed. cc @zhiics @mbarrett97

[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass

soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586529239 This test is now failing at the de-duplicate pass. It's interesting since the "correct" graph has the

[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass

soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586527973 Definitely, I'll do that after fixing this.

[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass

soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586527594 Thanks everyone for working through this with me!

[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass

soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586527376 This is actually a good exercise, as some other tests are actually failing now due to the graphs not

[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass

soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586525810 If I flip the arguments to alpha_equals and properly add the attributes to the "expected" function, the

[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass

soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586521847 Actually, are you sure the second argument is expected? It looks like AlphaEqual loops through the LHS

[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass

soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586515076 I think so. If everyone else is okay, I think it's best to check this fix in.

[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass

soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586512825 Sorry, what exactly do you mean? This is

[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass

soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586509270 That's really strange - it looks right. Are you able to pull my branch and give the test a try?

[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass

soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586472422 I'll do some investigation :) This is an

[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass

soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586469382 graph_equal also succeeds on the buggy graph :( @zhiics, let me know when you have any findings!

[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass

soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586428127 Yes, they both pass the alpha_equal check.

[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass

soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586412142 @mbarrett97 @comaniac I just pushed a test where "result" creates an incorrect graph, and "expected" is

[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass

soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586044128 Sure, I'll work on adding a unit test.