[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-577930418 so tvm will finally compile itself :) didn't know self hosting is one of our goal 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: us...@infra.apache.org With regards, Apache Git Services
[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-577882904 Choosing cpp vs python per operator (rather than one lang for all ops) is a good compromise, but it creates other problems (need to discuss which impl to keep for each op, not obvious to newcomers which lang a particular op is implemented in, etc). But we can talk about that later. So, going back to this PR, I propose to remove cpp resize and upsampling because, * resize op is highly unstable. Recently people added new functionalities to resize python to support ONNX or Tensorflow use cases. ONNX, the one I'm familiar with, has a dozen of options that we have no support for. It is likely that we need to update our resize definition to cover more new or esoteric use cases from higher level frameworks / standards. resize cpp hasn't got any update and it can only do nearest/bilinear resize. * upsampling cpp is useless and it simply forwards to resize cpp. If we remove resize cpp, it is automatically removed. 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: us...@infra.apache.org With regards, Apache Git Services
[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-577882904 Choosing cpp vs python per operator (rather than one lang for all ops) is a good compromise, but it creates other problems (need to discuss which impl to keep for each op, not obvious to newcomers which lang a particular op is implemented in, etc). But we can talk about that later. So, going back to this PR, I propose to remove cpp resize and upsampling because, * resize op is highly unstable. Recently people added new functionalities to resize python to support ONNX or Tensorflow use cases. ONNX, the one I'm familiar with, has a dozen of options that we have no support for. It is likely that we need to update our resize definition to cover more new or esoteric use cases from higher level frameworks / standards. resize cpp haven't got any update and it can only do nearest/bilinear resize. * upsampling cpp is useless and it simply forwards to resize cpp. If we remove resize cpp, it is automatically removed. 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: us...@infra.apache.org With regards, Apache Git Services
[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-577882904 Choosing cpp vs python per operator (rather than one lang for all ops) is a good compromise, but it creates other problems (need to discuss which impl to keep for each op, not obvious to newcomers which lang a particular op is implemented in, etc). But we can talk about that later. So, going back to this PR, I propose to remove cpp resize and upsampling because, * resize op is highly unstable. Recently people added new functionalities to resize python to support ONNX or Tensorflow use cases. ONNX, the one I'm familiar with, has a dozen of options that we have no support for. It is likely that we need to update our resize definition to cover more new or esoteric use cases from higher level frameworks / standards. resize cpp haven't got any update and it can only do nearest/bilinear resize. * upsampling cpp is useless and it simply forwards to resize cpp. If we remove resize cpp, it is automatically removed. 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: us...@infra.apache.org With regards, Apache Git Services
[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-577882904 Choosing cpp vs python per operator (rather than one lang for all ops) is a good compromise, but it creates other problems (need to discuss which impl to keep for each op, not obvious to newcomers which lang a particular op is implemented in, etc). But we can talk about that later. So, going back to this PR, I propose to remove cpp resize and upsampling because, * resize op is highly unstable. Recently people added new functionalities to resize python to support ONNX or Tensorflow use cases. ONNX, the one I'm familiar with, has a dozen of options that we have no support for. It is likely that we need to update our resize definition to cover more new or esoteric use cases from higher level frameworks / standards. * upsampling cpp is useless and it simply forwards to resize cpp. If we remove resize cpp, it is automatically removed. 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: us...@infra.apache.org With regards, Apache Git Services
[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: us...@infra.apache.org With regards, Apache Git Services
