tracel-ai / burn

Burn is a new comprehensive dynamic Deep Learning Framework built using Rust with extreme flexibility, compute efficiency and portability as its primary goals.
https://burn.dev
Apache License 2.0
8.66k stars 430 forks source link

Extract onnx importing into it's own crate. #1812

Open skewballfox opened 4 months ago

skewballfox commented 4 months ago

Feature description

I think the files in crates/burn-import/src/onnx/ listed below, along with the corresponding onnx_test, should be separated into their own crate:

Feature motivation

I think it would be both beneficial to burn and multiple other rust DL projects to create a generalized onnx library, and burn is most of the way there. All of the code under those files are currently solving the general problem of parsing an Onnx graph. The burn-specific conversion doesn't happen until to_burn.rs.

The benefit to burn is more contributors for that specific part of the op support and we could work ahead of the actual op support burn-side. We(I) could actually go ahead and generate all (or most) of the onnx models for the ops, along with the corresponding onnx test.

(Optional) Suggest a Solution

The test would have to be changed somewhat, either using macros or traits to make it to where libraries could drop in there tensor types, or storing the expect input/output into json or npy files in the same directory as the onnx file, along with a couple of macros for easy test generation.

I'm not sure whether the crate should stay under the burn repo, or be separated out completely

Also if you are open for suggestions to non-burn names, I suggest SardOnnx (after the red-colored variant of onyx)

nathanielsimard commented 4 months ago

I think we can still keep the crate in the burn repo, since it's gonna be easier to update it, but we can give it a name without the burn prefix, something like onnx-loader

antimora commented 4 months ago

This is a great idea. From the beginning I was intending Burn's ONNX IR to be its own crate. For now, however, we should keep it within the burn repo with a non-burn name. We are still making changing to the IR. In addition to missing ONNX node types, we need to add three big features: Subgraphs (#724), functions (#723) and scalar graph inputs (#1688).

Once we separate ONNX IR, we should put forward a design document that clearly states design goals that works for other projects as well.

Here are the names, I would like to suggest. It would be great if we somehow convey an idea that we are unifying ONNX representation from various OpSets into one.

  1. onnx-unify (I prefer this).
  2. onnx-north
  3. onnx-irgen
  4. onnx-nexus
  5. onnx-simplify
  6. onnx-unite
  7. onnx-parse
  8. onnx-clarity
  9. onnx-unified
  10. onnx-ir
  11. onnx-easy
  12. onnx-bridge
  13. onnx-harmony
  14. onnx-meridian
  15. onnx-fusion

@skewballfox @laggui @louisfd @nathanielsimard what do you think about the names?

laggui commented 4 months ago

I think onnx-parse is simple and clear to the point. onnx-unify could also work but I think it's not as clear.

nathanielsimard commented 4 months ago

Same as @laggui, but I also like onnx-ir!