Glow: [RFC] Removing non-custom ONNXModelWriter logic/support

[jfix71]

Now that we use our own custom Glow ONNX serialization, I'm wondering about the value of the original ONNX serialization logic.

For context, this was originally intended to serialize models to ONNX from Glow IR. However there's not always a 1:1 match from Glow Nodes to official ONNX ops. So we added some custom Glow ops to support this by hand, which kind of made the idea of abiding by the official ONNX spec pointless. Additionally there was always some bugginess when (de)serializing due to mismatches in IR representation. To improve this I added fully custom Glow ONNX serialization which uses essentially no official ONNX ops (only Identity) and strives for identical function serialization and bitwise identical results.

The original ONNX serialization logic requires a ton of code that adds complications for the custom serialization and in general extra complexity/potential for bugs. And I'm not sure the original ONNX serialization is used by anyone or useful at this point.

Does anyone use it that would object to removing it? We would of course could keep around the same support in the ONNXModelLoader. So any previously serialized models that used to work could still be loaded same as before.

CC: @opti-mix @yinghai @mciprian13 @aturetsk

[mciprian13]

@jfix71 I never used the ONNXModelWriter and never planned to so no objection here.

[yinghai]

SGTM

[saurabh-shandilya]

@jfix71 I briefly explored using this functionality to use glow as an optimization tool for onnx models to get an optimized onnx model, as I find lack of support for GPUs somewhat limiting for some of my usage. I was planning to use this feature but was not sure about 1 to 1 matching between glow nodes and onnx ops, but I was thinking for most of the trivial classification networks should have worked fine, unless my understanding is not correct. I'd prefer if glow can't support GPU support, it should be kept an maintained, as folks interested in GPU based runtimes can still make use of some useful parts of glow, if facebook indeed considers this as a useful usecase.

[jfix71]

Hi @saurabh-shandilya -- I agree for simpler networks it should be less of an issue to use ONNX export, since it should be easier to map back to real ONNX ops. You said you explored it -- did you actually get it to work?

Also curious what GPU were you want to run on. We do have some OpenCL support but of course this may not suit your needs.

[saurabh-shandilya]

@jfix71 I want to run it on nvidia Quadro P2000 but basically a particular GPU is not important. I think it did work fine for one simple model I tried.