-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Bug] [Relax] Missing IR structure checking and correction #17211
Comments
Good catch, and I think this is arising from a number of different edge cases.
I think there's a number of improvements that could be made, in order to close each of these loopholes.
I think all of these would be useful changes to make, but some would have wider impacts than others. The well-formed checks could be added with the smallest risk of breakage, but also place the greatest load on new developers. Improved normalization would provide the greatest ease-of-use, but would require the most widespread changes. @tqchen, since some of these would be much more involved changes, do you have preferences/thoughts on them? |
A similar bug occurs as shown below. Actual behavior
Steps to reproduce
|
Hmm. I think this is something that should be catchable by propagating the known struct info, but currently isn't caught.
I think this is a limitation in the
For the example, this would let the |
And one step implemented which should make it harder for these inconsistent shapes to emerge. In #17216, the |
Hi all, I set
check_well_formed=True
in the below Relax IR construction and can runmod.show()
to show the IR successfully. It seems the Relax IR passed the legitimacy checking. However, the compilation crashed when executingex = relax.build(mod, target='llvm')
. The crash message shows that"Argument 0 type mismatch: expected R.Tensor((16,), dtype="float32"), given R.Tuple(R.Tensor((16,), dtype="float32"))"
Based on my analysis, if we replace the code
gv1 = R.call_tir(cls.relu, (x), out_sinfo=R.Tensor((1, 512, 64, 64)))
(Line 26) withgv1 = R.nn.relu(x)
(Line 27) orgv1 = R.call_tir(cls.relu, (x,), out_sinfo=R.Tensor((1, 512, 64, 64), dtype="float32"))
(Line 28), the script can run well.Even if the Relax IR constructor can convert
gv1 = R.nn.relu(x)
to full information with type based on the context, why didn't it complete the missing type forgv1
(Line 26).To take a step back, if the Relax IR constructor cannot complete the missing information and we set
check_cell_formed=True
in the Relax IR construction, we should throw an exception early inmod = Module
rather thanrelax.build()
. Early crashes will make the code more robust.BTW, I prefer the IR constructor can fill in missing information or correct the inconsistent constraints based on IRs' context.
Actual behavior
Environment
Steps to reproduce
cc @Lunderberg @junrushao @tqchen
The text was updated successfully, but these errors were encountered: