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
9.01k stars 446 forks source link

ONNX: Implement Equal operator #557

Closed willstott101 closed 1 year ago

willstott101 commented 1 year ago

Feature description

It'd be great if the onnx import could support the Equal operator.

Feature motivation

Version 4 of the Silero VAD model uses this, FYI no rust runtime supports the latest silero model atm, so movement in that direction would be pretty cool :)

I'm sure there are plenty more operators or features missing, but making an issue for the first one.

Relates to: #322 Other silero issues in the rust ecosystem: https://github.com/sonos/tract/issues/1029

nathanielsimard commented 1 year ago

@Luni-4 implemented the equal operator, and its merged on main: https://github.com/burn-rs/burn/commit/67ce5ec62cb0e431c0353edd015ef534bef3abe6.

willstott101 commented 1 year ago

Oh cool! Thank you all for your hard work. However I still get the same error message - maybe nodes are different to operators?


will@will-laptop:~/repos/misc/experiments/rust-vad$ cargo build
   Compiling vad v0.1.0 (/home/will/repos/misc/experiments/rust-vad)
error: failed to run custom build command for `vad v0.1.0 (/home/will/repos/misc/experiments/rust-vad)`

Caused by:
  process didn't exit successfully: `/home/will/repos/misc/experiments/rust-vad/target/debug/build/vad-cd82745b2cee2ca8/build-script-build` (exit status: 101)
  --- stdout
  [DEBUG - /home/will/repos/others/burn/burn-import/src/onnx/from_onnx.rs:346] Found ONNX node type => Equal
  [ERROR - /home/will/repos/others/burn/burn-import/src/logger.rs:30] PANIC => panicked at 'called `Result::unwrap()` on an `Err` value: VariantNotFound', /home/will/repos/others/burn/burn-import/src/onnx/from_onnx.rs:319:73

  --- stderr
  thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: VariantNotFound', /home/will/repos/others/burn/burn-import/src/onnx/from_onnx.rs:319:73
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
will@will-laptop:~/repos/misc/experiments/rust-vad$ git -C /home/will/repos/others/burn/ log
commit 6b459fde8282a46d3c8848091676bc384b07445f (HEAD -> main, origin/main, origin/HEAD)
Author: Dilshod Tadjibaev <939125+antimora@users.noreply.github.com>
Date:   Fri Jul 28 11:52:23 2023 -0500

    Update Conv2d source generation to support all attributes (#558)

commit 7dbd43d111224ff9881e026cf5629294122914f5
Author: Luni-4 <luni-4@hotmail.it>
Date:   Fri Jul 28 18:51:41 2023 +0200

    Rewrite `publish.sh` script in Rust language (#556)

commit aa4af29e3f877aea25a7676eada7aa23bb3edc6c
Author: Louis Fortier-Dubois <louisfd94@gmail.com>
Date:   Fri Jul 28 10:41:27 2023 -0400

    Matmul speedup (contiguous load) (#559)

commit 67ce5ec62cb0e431c0353edd015ef534bef3abe6
Author: Luni-4 <luni-4@hotmail.it>
Date:   Fri Jul 28 01:37:10 2023 +0200

    Implement binary operators for `burn-import` (#532)
antimora commented 1 year ago

Reopenning it to investigate closely.

antimora commented 1 year ago

@willstott101 if it's possible, could you please share the link to the ONNX file or upload if it's possible to do so? It will help to troubleshoot.

Please, note, burn-import still being developed and there are many ops are missing. ONNX is complex, and our strategy has been to move forward by adding features required by a concrete ONNX file instance, instead of going by the ONNX spec and try to cover all cases, which isn't practical.

Thanks for using and reporting the issue.

willstott101 commented 1 year ago

Of course! https://github.com/snakers4/silero-vad/blob/82d199ff22ae8897946ebb374d216cf6ce4f9aa3/files/silero_vad.onnx

Bear in mind that this is possibly quite an unusual model as I have struggled to find any libraries other than onnxruntime that can actually use this onnx file. I don't know if that makes it a good example for burn at this stage or a bad one? :laughing:

If I have time I might tinker with adding some operators it uses, but i'm really just taking a look at this for curiosity's sake. It's a model I deploy at work with onnxruntime on desktop intel cpus, and I just like the idea of being able to use vulkan and rust to speed it up.

antimora commented 1 year ago

@willstott101, thanks for sharing. I think have found an issue. The Equal implementation done earlier was with tensors but the example you shared is with scalars. I currently working on improving burn-import's binary operators to support scalars. So hopefully with that it'll fixed. However, your next problem is that your graph uses two major architectural features that missing from burn-import: branching and subgraphs, these are double with Burn but requires a bit of refactoring. Basically, burn-import will need to multi-graph to multi-model conversation. Currently, burn-import assumes a single graph and it generates a single model.

The top graph from your example: image

One of the subgraphs (note this subgraph contains LSTM which is supported by burn but missing from burn-import): image

The number of subgraphs:

image

Yes, your ONNX will be challenging to make it work but it's completely doable with Burn.

Tagging @nathanielsimard and @Luni-4 so they're aware of the use cases.

Note: I used Netron to view your ONNX file: https://netron.app/

antimora commented 1 year ago

@willstott101 I have filed a ticket to support subgraphs: https://github.com/burn-rs/burn/issues/724