[TOPI] Remove cpp upsampling and resize op

[masahi]

We have both Python and C++ implementation of upsampling op. Until recently the C++ implementation was being used by NNVM, but now that NNVM is gone, we should remove it.

Otherwise it only creates confusion. There was a time when people tried to port python topi to c++ to reduce dependency on python. But nowadays it doesn't seem to be the focus of people.

The situation of resize is similar but worse. People added more functionality to python resize (more coordinate transform, bicubic, crop_and_resize etc.) and nobody updated the c++ version.

We can also discuss if we want to remove upsampling op entirely, since it just forwards to the resize op. Historically, upsampling was first added in #772 as one of "the standard neural network operators". But later somebody added resize op to support Tensorflow's image operation. I thought (and still think) it is weird to introduce "image" operations inside network, but by now ONNX also deprecated Upsampling in favor of Resize, so this seems to be the standard. (Although people claim Resize is more general than Upsampling, it's funny pretty much most use cases for Resize is to do upsampling).

please review @vinx13 @jwfromm @yzhliu @icemelon9

[jwfromm]

LGTM, thanks for taking the time to finally cull these out! In regards to your point about upsample vs resize, I agree they're redundant and am in favor of eventually dropping upsample. However, that would require a bunch of changes to frontends and tests and might be best suited for another PR.

[tqchen]

In the case of upsampling and resize though should we consider them as stable ops and keep only cxx impl instead ?

I agree it is bad to keep both versions, would be great if we can decide on a version to keep

[masahi]

I don't think C++ only solution is realistic so I prefer to keep only python. C++ resize in particular is hopelessly outdated and I don't want to be the one porting new functionalities from python. Even if I did that it wouldn't make my life any better.

People develop new stuff in python and it is not reasonable to ask them to port it to C++ when they send their PR. And after the python PR it is unlikely they would spend their time for C++ porting. Me and @jwfromm have been involved in development and review of resize op and we know what we are talking about :)

[tqchen]

I do not mean that we have do it in this PR. Indeed having a C++ impl would be harder than the python version.

On the other hand, it brings certain benefit to help the compiler to be more language agnostic, so this is a direction that we should strive to push for.

Given that the current cxx solution is incomplete, we could remove it, but if in the future we could have an effort to bring a stable cxx only solution, we might want to favor that one

[masahi]

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

[tqchen]

Hmm, i hear you. I think the nature of “need to adding new features” marks the op unstable, thus merit a python version as the primary one.

On the other hand, for most of the numpy style ops, whose interface are stable, it makes sense to encourage c++ version and use that as default(while removing the python version)

In both cases, for each op there is one primary language for the op, depending on the status. And we can remove the other one

[masahi]

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.

[icemelon]

I agree with @masahi. During the work of #4644, I found a few inconsistency between cpp and python topi implementation. Even for some simple schedule like injective, python version is better than cpp version, and has already overwritten the cpp implementation. I feel that keeping both cpp and python implementation for topi is a bit confusing, even that one op may only exist in cpp or python. Given that most of people now contribute to topi python, it makes sense that we only maintain the python version.

In response to @tqchen, op compute could be stable, but the schedule might not be. Even op compute doesn't change over the time, we can still improve its schedule for better performance. It's easier to do so in python than in cpp.

[tqchen]

We can have another discussion thread about the scope of ops that goes to cxx.

My current take is that the decision will likely be made as per group basis, eg all numpy ops could goes to c++ as numpy interface is pretty stable.

The same rule applies to schedule, so if a compute is stable but schedule is not, we can keep the schedule in py

[masahi]

will wait for others to give opinions, will merge tomorrow.

[tqchen]

btw, we all love python. Perhaps the final solution is we use tvm to compile these python descriptions XD so we don’t need to to have the debate. Thanks everyone for great inputs

[masahi]

so tvm will finally compile itself :) didn't know self hosting is one of our goal

[tqchen]

I am half joking but who knows, maybe we will get there