RFC: archive the current ONNX.jl implementation and replace with BaseONNX.jl's

[ToucheSir]

Relevant Zulip discussion: https://julialang.zulipchat.com/#narrow/stream/237432-ml-ecosystem-coordination/topic/ML.20and.20AD.20dev.20call

Looking at https://github.com/FluxML/ONNX.jl/issues, I think it's fair to say that ONNX.jl is unmaintained and does not function on remotely recent versions of Julia. As such, I propose revamping the implementation and creating a v0.2 with something that is maintained and actively used by members of the community. Currently, the best candidate is BaseONNX.jl, or more specifically @DrChainsaw's fork at https://github.com/DrChainsaw/ONNXmutable.jl/tree/master/src/baseonnx. This package only exports the protobuf definitions and model reading/writing, but at least it works!

I would add that this is also an opportunity to create a proper ONNX package instead of an "ONNX for Flux" implementation like ONNX.jl is now. @dfdx, @GunnarFarneback and @egeersu can probably provide some non-Flux use-cases.

[DrChainsaw]

Sounds great! One idea I have had in my head is the one ought to be careful with 'namespacing' so that e.g. someone tabbing ONNX in the pkg REPL does not get multiple options, most of which don't seem to do anything.

Perhaps this is a pointless (who installs a package without looking at the docs first?) and futile (as we can't control who registers packages) endeavour anyways and we should perhaps instead take the opportunity to make ONNX.jl useable and not locked in on one framework.

[darsnack]

We discussed this, and no one had objections to archiving the current ONNX.jl. I would say someone here can fork ONNX.jl and swap out the code with @DrChainsaw's baseonnx code. If there is a need to have a separate BaseONNX.jl and ONNX.jl, then we can have both under the org, and both can be updated at will.

[dfdx]

It would be great to agree on the API as well. The most high-level still framework-independent way to represent ONNX graph that I can imagine is, well, graph. Maybe some existing, maybe a brand new ONNXGraph with simplified API so that developers could easily grasp it and translate to a framework specific representation. Any thoughts or existing options for this?

[ToucheSir]

That can happen in parallel to this effort. I recommended moving over the bare minimum first so we're not blocked on design discussions about higher-level features for another year or what have you.

[DrChainsaw]

I agree with @ToucheSir but I think the proposal is valid.

Here is what I proposed for BaseONNX: opus111/BaseOnnx.jl#2

Rather than having some stubbed intermediate format, just give a utility to build any kind of graph from the protos.

[dfdx]

Agree with both points. So what are the next steps? Should we simply replace the contents of ONNX.jl with files in https://github.com/DrChainsaw/ONNXmutable.jl/tree/master/src/baseonnx and update the README?

[DrChainsaw]

Should we simply replace the contents of ONNX.jl with files in https://github.com/DrChainsaw/ONNXmutable.jl/tree/master/src/baseonnx and update the README

I think that is the proposal. I can do this whenever I have the time and energy.

If someone else wants to do it then I'd be happy to get off the hook, no need to bother about preserving authorship or anything. I guess that things like writing the readme would be easier for me though as I have written the non-autogenerated parts and I'm probably the only person who has used it. I would also like to add documentation to all functions which is not much work for me once I get around to it.

[dfdx]

Cool! Then I'm going to reserve a couple of hours this weekend and create the PR.

[dfdx]

Here we go: FluxML/ONNX.jl#46
There's a couple of questions / doubts there, don't hesitate to add your opinion.

[DrChainsaw]

Great! Thank you so much for doing this. Sorry for not being more active here. I don't have any power to make the pull happen unfortunately. @darsnack or @ToucheSir, what is the next step?

Should we make some extra effort to not step on any toes given things like links to blog and paper or is that sorted out already?

[DrChainsaw]

Should we make some extra effort to not step on any toes given things like links to blog and paper or is that sorted out already?

Ok, I see now that the ball is rolling w.r.t this in the PR. Just ping me if you need anything. I can try to find the time to write a decent readme and make a PR with improvements to docstrings once this gets merged.

[ToucheSir]

Seems like everyone is fine with the changes, so if you don't hear back in a couple days feel free to ping someone with merge rights (and myself, if you need an argument for pushing things through. You probably won't though 🙂)

[dfdx]

Now when FluxML/ONNX.jl#46 is merged I believe we can move forward and talk about more high-level machinery. I have a couple of ideas, but given all the prior effort (e.g. this thread) I guess quite a thought out plan already exists (even if it's not agreed on yet). If so, I'm ready to jump in and provide the workforce.

[Moelf]

How is this going?

[dfdx]

I just came back to ONNX.jl and plan to spend the next month on it. At the moment I'm working on importing HuggingFace's BERT, which included around 10-15 operators we don't yet support. Most operators are quite simple and only need proper testing, but there's a couple of tricky ones (Gather is bending my mind right now!).

[darsnack]

We do have NNlib.gather or is the translation to our method the tricky part?

[dfdx]

The funny thing is that ONNX has Gather, GatherND and GatherElements, and NNlib.gather is not the same operator as Gather, which I need for BERT.

But even without this difference, there's a lot of details. For example, Gather takes indicies as the second argument, and indices in Python and Julia work very differently! And many more details to keep in mind. So I'm moving forward, but things like this take most of the time in practice.

[lorenzoh]

Just want to say it's really cool you're working on this and I'm excited about the eventual interoperability with other ecosystems we'll get!