Lunderberg commented on code in PR #16757:
URL: https://github.com/apache/tvm/pull/16757#discussion_r1534347868
##########
python/tvm/relax/frontend/nn/exporter.py:
##########
@@ -121,7 +122,6 @@ def _effects() -> typing.List[typing.Tuple[str,
core.Effect]]:
outputs = _emit_effect_init(self.builder, effects)
self.builder.emit_func_output(outputs, params=[])
for method_name, method_spec in zip(spec.method_names,
spec.method_specs):
- params = _params() # Re-initialize so symbolic shapes not
shared across methods
Review Comment:
Unfortunately, generating fresh Relax variables for the parameters breaks
any expressions that have been defined in terms of the previous Relax
variables. (See [this test
case](https://github.com/apache/tvm/pull/16757/files#diff-5c2e0984ad1260219560ecdc3bfccb77f92e31ba26246a3ac4e128810c931e71R316)
for an example.) The expressions held internally to represent the
pre-processed weights are specified in terms of those initial Relax variables,
so replacing them with fresh Relax variables leaves the original variables
undefined. Using `copy_with_new_vars` afterward provides unique symbolic
shapes in each function, without breaking the expressions used within each
function.
Long-term, I'm planning to expand the `tir.transform.ConvertSSA` pass to
handle both TIR and Relax functions. That way, it can be applied as a
post-processing step to ensure all symbolic variables are unique to a single
function, without needing to handle it before that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
