[GitHub] [incubator-tvm] tqchen commented on pull request #5779: Improve separation of PyTorch dtypes and TVM dtypes in relay PyTorch frontend

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

tqchen commented on pull request #5779:
URL: https://github.com/apache/incubator-tvm/pull/5779#issuecomment-643499965


   It would be great to have a constructive discussion about the technical choices, agree on the pros and cons, before we reach a conclusion. Everyone is contributing to a common project (and we value everyone's opinion) and I think it would be great if we can have a clear discussion.  We also need to acknowledge that engineering decisions have tradeoffs and there is no true answer to the problem. 
   
   One common way I find that I find useful, is to dissect the discussion, label each discussion points, try to agree on sub points and rationales.
   
   In the conversation so far, I see a few choices:
   
   -  Ways to handle names:
        - T0: Allow optional input names from torchscript.
        - T1: Only user to pass in name override
        - T2: T0 and optionally allow T1
   - Ways to handle data type
        - D0: Assume most things are fp32
        - D1: Being able to convert the right data type from torchscript.
   
   We can then discuss their pros and cons. For example
   
   T0 is certainly more convenient, but it also depends on the stablity of the torchscript's ability to keep names. T1 is more explicit when a user intend to name the input.
   
   D0 solves most of the common problems, but as the machine learning models move to mixed precision, we will inevitably want to support more data types, that likely makes D1 more appealing.
   
   Because the pros and cons are mainly technical, I hope that most of us can agree on the technical points. The main thing that we might not agree on, would be something like the priorization of technical tradeoffs.
   
   For example, I might favor clear naming scheme over implicit and thus prefer T2. A different person might think simplicity is key and fp32 is fine, so D0 is OK. This should be the only part we disagree on. 
   
   When we find more comon grounds, it is much easier to reach agreements. In the cases as this, one thing we can do is to have a constructive discussion, and perhaps bringing up more people to see what everyone's thoughts. In many cases we can find that we do not disagree that much after all. It could be a good discuss forum thread.
   
   Regardless of the outcome, I want to say that we value good technical debates and usually they leads to better code overall. Many parts of this PR are certainly valuable, like the better data type handling. So let us have a good conversation and bring a better Pytorch support for everyone.
   
   
   
   
   
   
   


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