sonos / tract

Tiny, no-nonsense, self-contained, Tensorflow and ONNX inference
Other
2.24k stars 214 forks source link

Silero-vad-4 needs actual If operator #1029

Open 7r3nzy opened 1 year ago

7r3nzy commented 1 year ago

I am trying to load silero-vad-v4 onnx but it is giving me the following error:

Error Failed analyse for node #74 "If_25" If

Caused by:
    Wrong nnumber of outputs. Op says 1, node says 3.

This is how I am loading it:

        onnx()
            .model_for_path(model_path)?
            .with_input_names(["input", "sr", "h", "c"])?
            .with_input_fact(
                0,
                InferenceFact::dt_shape(f32::datum_type(), tvec!(1, voice_detection_frame_size)),
            )?
            .with_input_fact(1, InferenceFact::default())?
            .with_input_fact(
                2,
                InferenceFact::dt_shape(f32::datum_type(), tvec!(2, 1, 64)),
            )?
            .with_input_fact(
                3,
                InferenceFact::dt_shape(f32::datum_type(), tvec!(2, 1, 64)),
            )?
            .with_output_names(["output", "hn", "cn"])?
            .with_output_fact(
                0,
                InferenceFact::dt_shape(f32::datum_type(), tvec!(1, 1)),
            )?
            .with_output_fact(
                1,
                InferenceFact::dt_shape(f32::datum_type(), tvec!(2, 1, 64)),
            )?
            .with_output_fact(
                2,
                InferenceFact::dt_shape(f32::datum_type(), tvec!(2, 1, 64)),
            )?
            .into_optimized()?
            .into_runnable()

model overview

2023-04-03_10-00

If_25 node is the one with 3 outputs

tract version: 0.19.8

Update:

Running through command line has the same behavior:

tract silero-vad/files/silero_vad.onnx
┏ 1 Source sr
┃   ━━━ I64
┣┻ 73 Equals Equal_23
┃   ━━━ Bool
┃┏ 3 Source c
┃┃   ━━━ 2,batch,64,F32
┃┃┏ 2 Source h
┃┃┃   ━━━ 2,batch,64,F32
┃┃┃┏ 0 Source input
┃┃┃┃   ━━━ batch,sequence,F32
┣╋╋┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻┻ 74 If If_25
      ━━━ batch,1,F32
      ━━━ 2,batch,64,F32
      ━━━ 2,batch,64,F32
[2023-04-10T00:30:01.961133391Z ERROR tract] Error at stage analyse

    Caused by:
        0: Failed analyse for node #74 "If_25" If
        1: Failed analyse for node #74 "If_25" If
        2: Wrong nnumber of outputs. Op says 1, node says 3.
7r3nzy commented 1 year ago

Location where the comparison takes place:

https://github.com/sonos/tract/blob/bdb38a6b5e1cb10538e2d364512cef728ba16ae2/hir/src/infer/analyser.rs#L97

https://github.com/sonos/tract/blob/bdb38a6b5e1cb10538e2d364512cef728ba16ae2/hir/src/infer/ops.rs#L87

It seems like Op always returns 1.

@kali is it something that's doable? if so I'd be interested to make an attempt on it :).

Also I looked at the tract trace and it seems like it is able to analyze the whole model except the If operator.

7r3nzy commented 1 year ago

what if in onnx/src/ops/logic.rs we do something like this for If:

  fn nboutputs(&self) -> TractResult<usize> {
        if self.else_body.outputs.len() == self.then_body.outputs.len() {
            Ok(self.then_body.outputs.len())
        } else {
            //Some Err()
        }
    }
7r3nzy commented 1 year ago

It seems like I might have ditched that If error temporarily then I ran into this one :)

  Caused by:
        0: Infering facts
        1: Failed analyse for node #121 "Cast_173" onnx.Cast
        2: Eager eval during inference
        3: Evaluating #1 "adhoc" onnx.Cast
        4: Unsupported cast from TDim to Bool

It is coming from then_branch could be a separate issue related to Cast Op:

2023-04-10_09-59

kali commented 1 year ago

Hello! thanks for your interest and sorry for delay in answering.

The nboutput fix feels right. Feel free to make it a PR. You can use the ensure! macro to perform the test (search in the current codebase for inspiration).

I will have a look at the TDim to bool cast.

7r3nzy commented 1 year ago

Awesome! Thanks :)

kali commented 1 year ago

tract does not have an actual If operator at this stage (it expects the boolean condition to be resolved at load time, and substitutes itself with one branch accordingly). I'm putting this in the backlog for now.

garyhai commented 1 year ago

Awesome! Thanks :)

@7r3nzy I noticed that this is a reopened issue. Can you confirm if you are able to load the Silero model successfully on your end?