[GitHub] [tvm] comaniac edited a comment on pull request #7564: [BYOC] Exclude external params from Graph Runtime

[GitBox <gi...@apache.org>]

comaniac edited a comment on pull request #7564:
URL: https://github.com/apache/tvm/pull/7564#issuecomment-789105015


   `ConstantUpdater` can be customized as introduced in this #6697, so the parameter naming convention is not guaranteed (cc @zhiics, does `codegen_name` and `symbol` already the same?) Even it's the agreement, I still have the following two concerns:
   
   1. The naming convention can be changed anytime, and seems like we have no way to know that we need to change this part accordingly. Specifically, we need a test case if we really need to use the parameter name to decide whether to put the constants to metadata module.
   
   2. If a customized ConstantUpdater doesn't assign "null", then the intention is still maintaining the constants in the metadata module.
   
   In summary, a safer solution is to have a way to check whether the constant is alaready stored in the extenral module when building a module, but it's not easy and that's part of the reasons that we haven't fixed this issue until now.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org