tqchen commented on PR #16531: URL: https://github.com/apache/tvm/pull/16531#issuecomment-1978725128
Thanks @chunit-quic for the contribution! I think there are several goals: - G0: Support translation of PT ops, regardless if they appear in normal FX trace for ExportProgram - G1: Support ExportProgram usedcase - G2: Support integration of torch dynamo via FX graph I think we all agree that G1 is super important. I would like us to think about how we can unify, so we don't have two versions of the torch importer. Here is one possible way to do so: - We build a common base class that contains the translation of ops (that can be used in both fx and ExportedProgram) - The base class allows abstract method for program specific behaviors - `self.get_parameters(mod)` will obtain the parameters - For FXGraph we get from `model.named_parameters()` - For ExportedGraph we change according - `self.get_node_attrs(node)` will obtain the attributes - For - Support both op variants if needed - Because op keys are different, we can support `aten.conv2d` and `conv2d`, either as a subclass patch, or support both in base, it does not hurt since both can be useful at different levels, especially for key ops like conv2d -- 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: commits-unsubscr...@tvm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org