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

Silero-vad-4 needs actual If operator #1029

Open
7r3nzy opened this issue Apr 3, 2023 · 7 comments · Fixed by #1036
Open

Silero-vad-4 needs actual If operator #1029

7r3nzy opened this issue Apr 3, 2023 · 7 comments · Fixed by #1036

Comments

@7r3nzy
Copy link
Contributor

7r3nzy commented Apr 3, 2023

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

7r3nzy commented Apr 10, 2023

Location where the comparison takes place:

if outputs.len() != self.model.borrow().node(node).op.nboutputs().unwrap() {

fn nboutputs(&self) -> TractResult<usize> {

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

7r3nzy commented Apr 10, 2023

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

7r3nzy commented Apr 10, 2023

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

kali commented Apr 10, 2023

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

7r3nzy commented Apr 10, 2023

Awesome! Thanks :)

@kali
Copy link
Collaborator

kali commented Apr 10, 2023

  • The nboutput fix works.
  • fix cast to tdim #1037 fixes the cast problem.
  • there is still an issue (at least) with this model (silero v4)
  • "Can only deal with constant conditions in If translation"

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.

@kali kali changed the title Wrong number of outputs | Error on If operator Silero-vad-4 needs actual If operator Apr 10, 2023
@kali kali mentioned this issue Apr 10, 2023
5 tasks
@garyhai
Copy link

garyhai commented Nov 6, 2023

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?

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

Successfully merging a pull request may close this issue.

3 participants