tlopex commented on PR #19498:
URL: https://github.com/apache/tvm/pull/19498#issuecomment-4366032381

   Hi @swjng 
   Thanks for the fix! 
   
   However, `Concat._impl_v13` now calls `get_constant` on every input 
unconditionally. `get_constant` has a side effect: when the input is a param 
`Var`, it updates `graph_nodes[name]` to a baked `relax.Constant`.
   
   This fixes the shape-construction case, but it can also change the semantics 
of ordinary tensor concat under `keep_params_in_input=True`. For example, an 
ONNX graph with `Concat(x, weight)` should keep `weight` as a runtime 
parameter. With this change, `weight` may be folded into the IR as a constant, 
so the compiled function no longer depends on the runtime value passed for that 
parameter.
   
   Could we narrow the fix so param resolution is used only for the 
all-shape-like fast path, preferably without mutating `_nodes`? The tensor 
fallback should continue using the original `inputs`. It would also be good to 
add a regression test for normal tensor `Concat(input, initializer)` with 
`keep_params_in_input=True`, verifying that the initializer remains a real 
parameter input.


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to