You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/01/23 20:56:15 UTC

[GitHub] [incubator-tvm] masahi edited a comment on issue #4769: [TOPI] Remove cpp upsampling and resize op

masahi edited a comment on issue #4769: [TOPI] Remove cpp upsampling and resize op
URL: https://github.com/apache/incubator-tvm/pull/4769#issuecomment-577870143
 
 
   I do agree that the cpp solution is desirable and has many benefits. For operators whose api and implementation would not be expected to change (pooling, elemwise op etc) it makes sense to have them in cpp only (and that is what we have currently). I also liked the recent effort which brought most of the relay build pipeline to cpp. A few days ago I sent a [PR](https://github.com/apache/incubator-tvm/pull/4751) which exposed some relay cpp functionality to python. Now that things are in cpp, this "exposing" can be done for any language (at least in principle).
   
   C++ on topi is a different story :) I believe the cpp only solution is simply not realistic because of "human problem". We always say cpp is something "to strive for" or "to port over in the future", but it's never been a hard requirement. Also it is not clear what makes API "stable enough" to deserve a cpp impl. As soon as people find a reason not to do cpp, the cpp impl never happens. Most people who add features to topi just want to get their immediate jobs done, so "implement it in cpp" is usually not high on their list.
   
   This ambiguity has created situations that our cpp resize op is in. In addition to the "outdated cpp" problem, there are many topi operators that only exist in python, including 
   * all sparse ops
   * most of the vision specific ops (rcnn, ssd, deformable conv) except for the reorg op (for yolo)
   * int8 and other lower bit ops
   * framework specific ops (depth to space, fifo buffer etc)
   
   and I'm sure there are others.
   
   In summary,
   * I agree that the cpp only solution is desirable and something we should strive for, but in practice not realistic
   * Already too many inconsistencies on python vs cpp in topi. Most likely porting effort would not happen.  
   * The python only solution is the only viable option and basically it is already our status quo. We can keep the existing cpp impls that are useful, but we should make our python impl as the official one. This is also in line with the recent "Unified IR" initiative by @tqchen which, if I remember correctly, brings TVM to be more "Python first".
   
   I usually don't have a strong opinion on things, but I believe this topi python vs cpp duplication and inconsistency situation is the most ugly corner in our code base and so wanted to say something about it. I'd love to hear other opinions :) @jwfromm @yzhliu @icemelon9 

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


With regards,
Apache Git Services