oramasearch / onnx-go

onnx-go gives the ability to import a pre-trained neural network within Go without being linked to a framework or library.
https://blog.owulveryck.info/2019/04/03/from-a-project-to-a-product-the-state-of-onnx-go.html
MIT License
717 stars 71 forks source link

IR used by onnx-go is several version behind onnx #175

Open senkrish opened 4 years ago

senkrish commented 4 years ago

The current version of IR supported by onnx-go is 3, as defined here: https://github.com/owulveryck/onnx-go/blob/master/internal/onnx/ir/onnx.proto3.pb.go#L50

But onnx is at version 6, as defined here: https://github.com/onnx/onnx/blob/master/onnx/onnx.proto#L93

This is causing an issue when trying to import a model saved using sklearn Python module. I get the error:

Unknown input type: UNDEFINED

Changing the opset to make it backward compatible, as explained here does not seem to help: https://github.com/onnx/onnx/blob/master/docs/Versioning.md#released-versions

Are there any plans to upgrade the IR version supported by onnx-go?

owulveryck commented 4 years ago

Thank you for raising this issue and for the investigation you've made so far.


First, a bit of context: I started onnx-go a couple of months ago with the idea that we (Gophers) needed a simple way to handle a pre-trained neural net model within the Go ecosystem (and mainly in Gorgonia). I thought that the community could embrace the project and add contributions (TBH, I even though that it could, eventually, be moved to the onnx organization). As of today, it is still is a side project, and sadly, very few contributions are made to the project besides my own (even if the contributors have brought great value, and I am thankful for their work and time).


That said, I fully agree with you: onnx-go must have a proper policy to follow ONNX versions. But ONNX is moving fast (Microsoft, for example, is paying full-time excellent developers to work on onnxruntime and to various contributions to the IR), and as long as I won't have more time and/or more developers involved, I think this will be a hard task. Anyway, I am still thinking of a proper way to handle version compatibility for the package and any contribution or idea is welcome and appreciated.


Now about your problem (because I'd like you to be able to use the package :D): What I propose is to compile the latest IR with protoc and replace the code from the IR package with the new one, and if it remains compatible with the main package. But it's not a long term viable solution.

WDYT? Can you try that and see if it works in your case?

senkrish commented 4 years ago

I fully understand the constraints you are facing in getting mainstream adoption of this package, and I want to thank you for getting this started. Hopefully we will see more contributions going forward. My primary goal is to be able to run model predictions on Onnx models using Go, and the two main blockers to using onnx-go is this version compatibility issue and the lack of support for Onnx-ML operators (see https://github.com/owulveryck/onnx-go/issues/174).

I'm able to workaround the onnx-go limitations by using Microsofts onnxruntime C-Api and I'm calling these APIs from Go using Cgo, but then it has its own challenges of running it in production. But at least I have something that works.

About the issue at hand, your suggestion is exactly how I got the model import working - compiled the latest version of onnx.proto.3 and replaced the .pb.go file in my local workspace. The decoder still had issues (it is not able to parse an Onnx output node that is of type Map), but I temporarily worked around that to get the "model import" to succeed. And then I ran into the LinearClassifier not supported problem referenced earlier.