Skip to content
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

[Relax] Avoid wrapping TupleStructInfo into a Tuple for R.call_tir #17243

Merged
merged 2 commits into from
Aug 26, 2024

Conversation

Lunderberg
Copy link
Contributor

Prior to this commit, the different R.call_tir* variations would wrap the arguments into an in-line relax.Tuple, if it is not already a relax.Tuple. While this allows a tensor to be passed into these functions as a single argument (R.call_tir(func, arg, ...) instead of R.call_tir(func, [arg], ...)), the wrapped Relax variable may already refer to a tuple.

This use of a variable to refer to an argument tuple rather than an in-line argument tuple is not allowed by Relax. (See discussion on #15916 for details.) However, by wrapping a variable args: R.Tuple(R.Tensor, R.Tensor, ...) into a tuple-of-tuples, the error occurs after the expression has already been generated, and refers to an expression R.Tuple(R.Tuple(R.Tensor, R.Tensor, ...)) that doesn't appear anywhere in the user's input. This can make debugging difficult (see #17239 for an example).

This commit updates the argument-handling in R.call_tir to only generate an in-line relax.Tuple if the arguments do not already have relax.TupleStructInfo. If the argument was provided as a Relax variable bound to a tuple of arguments, it will still produce an error. However, that error will occur much earlier, and will explicitly state that the argument must be a relax.Tuple instead of a relax.Var.

Prior to this commit, the different `R.call_tir*` variations would
wrap the arguments into an in-line `relax.Tuple`, if it is not
already a `relax.Tuple`.  While this allows a tensor to be passed into
these functions as a single argument (`R.call_tir(func, arg, ...)`
instead of `R.call_tir(func, [arg], ...)`), the wrapped Relax variable
may already refer to a tuple.

This use of a variable to refer to an argument tuple rather than an
in-line argument tuple is not allowed by Relax.  (See discussion on
apache#15916 for details.)  However, by
wrapping a variable `args: R.Tuple(R.Tensor, R.Tensor, ...)` into a
tuple-of-tuples, the error occurs after the expression has already
been generated, and refers to an expression `R.Tuple(R.Tuple(R.Tensor,
R.Tensor, ...))` that doesn't appear anywhere in the user's input.
This can make debugging difficult (see
apache#17239 for an example).

This commit updates the argument-handling in `R.call_tir` to only
generate an in-line `relax.Tuple` if the arguments do not already have
`relax.TupleStructInfo`.  If the argument was provided as a Relax
variable bound to a tuple of arguments, it will still produce an
error.  However, that error will occur much earlier, and will
explicitly state that the argument must be a `relax.Tuple` instead of
a `relax.Var`.
@tqchen tqchen merged commit c4acc79 into apache:main Aug 26, 2024
19 checks passed
@Lunderberg Lunderberg deleted the relax_avoid_generating_tuple_of_tuples branch August 26, 2024 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants