Conv and KERAS storage_order does not match - is this right

[ed-mr]

Hi, is it right to say, that the operator Conv (used for Conv1D) in onnx does not support the supported storage_order "channels_last" (or "NWC"). Does it mean, that using onnx requires me to transpose the input data every time Conv1D is used?
Or is there a way to handle that without Transpose?
It looks like, that the Conv operator supports only the pyTorch setting?

[jcwchen]

Hi @ed-mr,
To simplify complexity, currently ONNX spec only allows NCHW. According to the operator document, generally ONNX uses NCHW format as input for Conv-family and Pooling-family operators. Although Keras and TensorFlow use NHWC by default, IMO, ideally the converter (tf2onnx) should handle this conversion (NHWC to NCHW) while converting to ONNX model without users' additional effort.

[ed-mr]

Hi @jcwchen,
I see the point and understand the operator documentation. As you write it, that answers the question but does not solve the problem. It is right to say, that tf2onnx handle it correctly according to the mentioned documentation. I see, that this is the pytorch way of handling Conv. For me and others around me (probably many tensorflow users) this incompatibility requires an additional transpose layer which is very costly on hardware resources (RAM and computational time.) Is there any way to raise a request to extend the language?

[jcwchen]

Sorry for the late reply because I spent some time understanding the whole picture. Although ONNX doesn't have native NHWC operators, such a model is still representable in ONNX. IIUC, currently the converter (tf2onnx) adds Transpose ops before operators which use NCHW and it will try to eliminate the transpose as much as possible. As you mentioned, a lot of Transpose is considerably costly in Runtime. To solve it, in the future ONNX will probably use local function to make something like "Transpose+Conv" as a single local function to grant backend the ability to accelerate with the function schema. With this approach, ONNX doesn't need to make a fundamental change to every related operators (like introducing an attribute to represent different channel format) or even introduce more operators to support it. Please note that such an idea is still ongoing.

[ed-mr]

thank you, that you had a second look at it and obviously digged into it.
I come from the tvm community: see https://tvm.apache.org
https://github.com/apache/tvm/blob/main/python/tvm/relay/frontend/onnx.py
I find the respective attribute "storage_order" which is supported on some other operator but not for conv.
I would simple add the attribute to the operator. The MaxPool-8 operator supports "storage_order" as attribute and it has the exact meaning. That would be a very simple solution to implement what I require.
If I use the same attribute with conv, model_checker works fine - but since it is not a supported operator, it is not supported by the runtime environment. I actually would prefer to have the attribute, since then, only the once who do implementation on this or that target have to take care.
(I checked in what I am talking about: https://github.com/ed-mr/onnx-issue/blob/main/my_keras_converter.py But that this fails with onnxruntime)

[ed-mr]

Thanks for your answer, but I can not understand it. You are saying "I think this approach provides more flexibility to the backend and also for some people who do not really need NHWC, they are not forced to take care of it." How do you mean, it is easier for a all frontends to find the (Transpose-X) pattern than to rewrite a simple loop order on the backend-side?
To my understanding It is a much harder job find the Transpose-X pattern and rearrange the parameter instead of switch the dimensional index in the backends. If that is to much job to do, the backend can use your favoured Transpose-X solution. Your choice require to lower the level of abstraction to a less abstract language in order to "reduce" the effort to implement it. Instead of keep the level of abstraction in the intermediate language and let the implanting backends let decide about the implementation.
Is it right to say, with ConvTranspose you already solved half the issue, why supporting TransposeConv? ConvTranspose it is decided on the basis on less effort for pytorch.

[jcwchen]

Thank you for providing your thoughts and I understand your concern. Before jumping into more details, let me briefly introduce model local function since ONNX does not have a document for it yet (#4120) -- Actually model local function solution means that the ONNX model will use a local defined function op NHWC_X to instead of Transpose+X in the graph and it will further have a defined local function (saying that NHWC_X represents Transpose+X) along with the ONNX model. For instance, having NHWC_Conv function op to represent Transpose+Conv. An example from tf2onnx: onnx/tensorflow-onnx#1910. Therefore, a backend can recognize such a local function op without putting it into ONNX standard.

Let me provide the perspective that why shouldn't directly push them into ONNX standard:

Instead of keeping the level of abstraction in the intermediate language and let the implanting backends let decide about the implementation.

As you mentioned, although ONNX published more operators or opset version bumps, backend still can decide whether to support them or not. However, it will have several drawbacks:

1. Even if backends choose not to support these updates, IIUC, they will still need to register new kernels for them without the detailed implementation to let backend recognize such new operators. It will potentially increase the binary size in every backend, especially if ONNX would like to support NHWC globally with a lot of operator updates.
2. If backends choose not to support these updates from standard ONNX, they will definitely receive complaints from users because they are "standard" operators.
3. More operators means more burden to newer ONNX backend supporter because they will need to handle more situations.

Due to the reasons above, the philosophy for ONNX right now is trying to make the standard operators as primitive as possible, to reduce the growth of operator number. For everything which can be represented as a function, ONNX will try to promote them: #3877. Thus going forward, ONNX will be more careful about introducing more operators or bumping version of current operators. Relevant discussion: #2872

Is it right to say, with #102 you already solved half the issue, why supporting TransposeConv? ConvTranspose it is decided on the basis on less effort for pytorch.

If ConvTranspose can be represented as a function op, I believe ONNX will try to promote it in the future. Also, IMHO, ConvTranspose is a more common operator in the entire AI world.

The open question here would be whether pushing those popular NHWC operators into ONNX standard. Or, in other words, it is a question that whether they are popular enough to make them standard and let front and backend support them. I think there isn't a final answer for it yet. At least for now model local function is a way to make it easier to represent as a function op in both front and backend instead of native Transpose+X.

[ed-mr]

Thank you, now I understand, that way you get new "operators" as local functions - which could or could not be implemented. Therefore, the NHWC (and NWC) local functions should be added to the list you mention : #3877. Right?

[ed-mr]

So, if you could place this list of NHWC local functions as to de done on the list #3877, I would be satisfied, if you find it valuable to keep the issue open for the ONNX standards discussions, I keep it open, otherwise close it.

[jcwchen]

To clarify, actually the list in #3877 is quite different from the local function case for NHWC here since they focus on promoting "existing" operators as function operators. Therefore they are originally ONNX "standard" operators. For the local function case here they are not ONNX standard operators originally and there is no plan to push them into standard, at least for now.

if you find it valuable to keep the issue open for the ONNX standards discussions, I keep it open, otherwise close it.

Feel free to label answered if you think you get a satisfied answer. I think people can still be able to discuss it in the same thread if they have questions. Thanks.

[zhenhuaw-me]

Thank @ed-mr for rounting me here. I did complain about ONNX's flexiblity of data layouts when creating tflite2onnx and spend much effort to resolve it. I agree that the NCHW constraint put burdens on the exporters and importers of ONNX. But two years later as of today, I think the NCHW constraints are acceptable.

ONNX is a protocal, a language, which needs to be understood by the exporters and importers. Understanding is a non-trivial effort.

As of now, the tools know the NCHW constraint and can easily generate or parse the semantic of the particular operator. They can insert Transpose to the local graph or do layout propagation across graph (tflite2onnx use the later approach). If the data layout are not the same, the complexity in the generated ONNX graph or the imported graph would increase. But these Transpose operators will eventually be optimized out in the backend framework since the layout manipulicating Transpose should cancel each other. For example,

1. Consider two NHWC Conv: [NHWC] - Conv - [NHWC] - Conv - [NHWC]
2. Converted to two NCHW Conv: [NHWC] - Transpose - [NCHW] - Conv - [NCHW] - Transpose - [NHWC] - Transpose - [NCHW] - Conv - [NHWC]
3. Cancel the Transpose: [NHWC] - Transpose - [NCHW] - Conv - [NCHW] - Conv - [NHWC]
   These graph optimizations should be implemented no matter whether it's limited by the ONNX standard - if the framework has progammable API, the end user might build such graphs.

So, the understanding effort as of now is inserting Transpose to the graph. The backends should optimize the graph.

Consider that we have storage_order or named dimensions in most layout sensitive operators. The exporters are likely to be easy since they can just fill up the attribution. The burden is on the importers side - they need to parse the attribution and insert Transpose to the graph if the backend representaion is not that flexable, which I think is the case for most inference projects. The backends should also be able to optimize out the uneeded Transpose if any in the graph. Maybe this is not always correct, if both the exporter and importer supports NHWC layout only, they don't need to insert Transpose - but still need to keep both NCHW and NHWC code path.

So, the understanding effort is inserting Transpose to the graph still (with the attribution in addition).

Seems not much benefit.

Another problem is that such change would create new version for base operators like Conv, which might not be widely supported by the ecosystem soon. That's why I changed my opinion - flexibility is good, and far more good if for free (no additional complexility).

I like the local function solution mentioned by @jcwchen which I think is really important to the ONNX ecosystem. I sometimes wondering why there are so many ONNX operators... TensorRT has put significant efforts to enable but there are more...

[zhenhuaw-me]

My point is, such Transpose elimitation graph optimizations should be introduced to inference systems regardless of the type of the input model. It's even more important for TVM since it has so many frontends. We would never know how the graph would be defined, end user can define such with TVM API.

[zhenhuaw-me]

I don't think the storage_order would happen soon even if the community decides to move towards it. The exporters need to upgrade too.

I used to create forks and implement whatever I need to get my problem solved. For example, when we were enabling TVM quantization (far before the official TVM) I created tensorflow fork to fix QAT issues on the r1.x branch, since the TensorFlow community focused on r2.x branch.

Maybe it will be more efficient for you to add something like a custom operator to enable the NHWC_Conv, and let the exporter and importer in your forks to recongize it . If this approach can solve your problem, I believe @jcwchen can help to route you to the related documents.

[ed-mr]

Thank you for your clear description and your proposal for some workaround. I just hopped to find strong support from the official side to support such issue and put it on the to do list. But just changing the import side of TVM is not the issue as such - pretty straight forward, since the operators are all available and for keras frontend, NHWC operators are already supported. The problem arises, since the operators are not accepted by the onnx checker and onnx runtime - and therefore, are not produced by no framework (as tf2onnx). The only solution is, as you describe, to find the "Transpose Conv" operators, substitute them and elimate the problem on the backend side. But my assumption is, that very same problem arises for others using onnx to support more frameworks. I got to understand, that the development of onnx does not really focuses on this community - the community with tough hardware restrictions.

Therefore, the best way to solve the problem is to have frontends for the individual frameworks - as TVM has for many - and not use onnx in the first place.

[zhenhuaw-me]

Please don't get me wrong. I though the action would vary depending on your first priority, either get the proposal in the ONNX, or get a model function on a device. Taking myself as example, if I'd like to get my model to work as soon as possible, I would turn to the custom operators (or any other path). That's why I shared how to do that.

On the other hand, if I committed to improve the ONNX protocal and the ecosystem, more actions need to be taken - it's much harder since it's not only a technical problem. I need to pursuade the community that the benefit of doing so is worthy, and maybe implement related features on my own.

Taking myself as example. I will be very happy if the PyTorch exporter can optimize on the ONNX graph before saving the model. Because in that way our ONNX importer can save some effort of hanlding the tricky shape manipulication subgraph. But clearly PyTorch team would spend their time somewhere else... If I really need that, I would need to do it by myself... You see, I don't have a sponsor of tflite2onnx, and I don't even use it. I created it because at that time I was interested in ONNX protocal and had experience of TensorFlow Lite. Everyone and every organizations have their priority, and they put efforts in the area themselves are most interested in, and share joint efforts if possible (intended or unintended). I guess that is how the community works.

Thanks for routing me here.

[ed-mr]

Just let me thank you both for taking your time discussing my simple question. I see very well, in the world of complex software you are pretty busy doing a lot of good stuff - people request and depend on. Therefore, thank you.