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

Reply via email to