Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
senkrish opened this issue Jan 16, 2020 · 2 comments
Open

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

senkrish opened this issue Jan 16, 2020 · 2 comments

Comments

@senkrish
Copy link

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

But onnx is at version 6, as defined here:
https:/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:/onnx/onnx/blob/master/docs/Versioning.md#released-versions

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

@owulveryck
Copy link
Contributor

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants