kraiskil / onnx2c

Open Neural Network Exchange to C compiler.
Other
184 stars 30 forks source link

Python port #19

Closed kernhanda closed 2 years ago

kernhanda commented 2 years ago

Hi - I ported your work to python. It's missing the quantization of models, as I felt it could be better served under a different utility (quantize and then onnx2c.py).

Rationale for the port:

Not sure if you'd like to contribute, but I'd be very interested in collaborating with you. :)

kraiskil commented 2 years ago

Interesting :) I had a quick look - am I correct in understanding that you have manually written all of node.py? I.e. it is not machine generated from the C++ sources of onnx2c? If so, we really should keep in sync with any bugfixes at a minimum.

This does sound like a lot of work though. Did you try writing python wrappers around the onnx2c_lib? This would of course not make the internals of onnx2c programmable in python, but could give 2 out of the 3 points you list above with maybe considerably less effort.

Dropping the quantization option sounds like a smart decision. I tried other quantizers before writing that, but they all seemed a bit work-in-progress at the time, and none would get rid of all the floats in the generated code. So I rolled my own - I fear it might be stable for that single example Arduino-MNIST project only. I guess with all the focus on quantization since then have made these other options much more viable.

kernhanda commented 2 years ago

I had a quick look - am I correct in understanding that you have manually written all of node.py? I.e. it is not machine generated from the C++ sources of onnx2c? If so, we really should keep in sync with any bugfixes at a minimum.

Yes, it's hand written. I'd be happy to sync up bug fixes.

This does sound like a lot of work though. Did you try writing python wrappers around the onnx2c_lib? This would of course not make the internals of onnx2c programmable in python, but could give 2 out of the 3 points you list above with maybe considerably less effort.

TBH, it hadn't occurred to me, since the native implementation is what I was trying to get away from. I didn't want there to be dependencies on the availability of a C++ toolchain, since that's considerably more of a quagmire than Python ecosystem (IMO, YMMV). So, I guess that's point number 4. :)

The effort was minimal, really. Your code is very well structured and documented. :) My work is also tangentially related to this, so we will be making use of this or a derivative.

Dropping the quantization option sounds like a smart decision. I tried other quantizers before writing that, but they all seemed a bit work-in-progress at the time, and none would get rid of all the floats in the generated code. So I rolled my own - I fear it might be stable for that single example Arduino-MNIST project only. I guess with all the focus on quantization since then have made these other options much more viable.

Yeah, it also seemed like a lot of work ;) It also seemed orthogonal to the project's goal, since quantization methods can vary so much. You can also have custom ops that deal with quantization, so any kind of model -> model transform should be a separate phase of the pipeline.

kraiskil commented 2 years ago

I didn't want there to be dependencies on the availability of a C++ toolchain, since that's considerably more of a quagmire than Python ecosystem (IMO, YMMV)

It does indeed vary :D Did you try to compile onnx2c on Windows or something? Though in hindsight, and indeed looking at your python version, writing this in python might have been the smart choice.