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

Fix progress when using VAD chunking #179

Merged
merged 2 commits into from
Jul 3, 2024
Merged

Conversation

finnvoor
Copy link
Contributor

@finnvoor finnvoor commented Jul 2, 2024

Previously progress would jump around as the same progress object was used for every chunk. Now every chunk is added as a child progress with equal unit count.

fixes #151

@finnvoor finnvoor force-pushed the main branch 2 times, most recently from 331cf70 to 8a1baa7 Compare July 2, 2024 14:54
@ZachNagengast
Copy link
Contributor

ZachNagengast commented Jul 2, 2024

This looks great, can you explain how the progress will look in the UI? In other words, on what event does the percentage increase? Maybe a video recording would help if you are able.

My current understanding is that each child's progress will be tracked independently and messaged back to the parent the total progress of each window—is that correct?

@finnvoor
Copy link
Contributor Author

finnvoor commented Jul 2, 2024

The progress update happens at the same time still, after each mel>encode>decode loop, but previously each transcribe task would go from 0->1 simultaneously, resulting in random updates.

Now each task is a child progress of the total progress, with an equal weighting, so the progress increases properly as each child progress goes from 0->1.

Before
Progress: 0.440
Progress: 1.000
Progress: 0.427
Progress: 0.758
Progress: 1.000
Progress: 0.768
Progress: 1.000
Progress: 0.758
Progress: 0.829
Progress: 1.000
Progress: 0.924
Progress: 1.000
Progress: 0.867
Progress: 1.000
Progress: 0.853
Progress: 1.000
Progress: 0.787
Progress: 1.000
Progress: 0.720
Progress: 1.000
Progress: 0.711
Progress: 1.000
Progress: 0.741
Progress: 0.905
Progress: 1.000
Progress: 0.711
Progress: 0.967
Progress: 1.000
Progress: 0.768
Progress: 1.000
Progress: 0.933
Progress: 1.000
Progress: 1.143
Progress: 1.000
Progress: 1.175
Progress: 1.000
Progress: 0.711
Progress: 0.791
Progress: 1.000
Progress: 1.027
Progress: 1.000
Progress: 0.915
Progress: 1.000
Progress: 0.806
Progress: 1.000
Progress: 1.280
Progress: 1.000
Progress: 0.806
Progress: 1.000
Progress: 0.910
Progress: 1.000
Progress: 0.834
Progress: 1.000
Progress: 1.043
Progress: 1.156
Progress: 1.000
Progress: 0.981
Progress: 1.000
Progress: 0.908
Progress: 1.000
Progress: 1.109
Progress: 1.000
Progress: 0.806
Progress: 1.000
Progress: 0.948
Progress: 1.000
Progress: 0.924
Progress: 1.000
Progress: 1.081
Progress: 1.000
Progress: 1.009
Progress: 1.000
Progress: 1.090
Progress: 1.000
Progress: 1.261
Progress: 1.000
Progress: 1.172
Progress: 1.000
Progress: 1.137
Progress: 1.000
Progress: 1.081
Progress: 1.000
Progress: 0.900
Progress: 1.000
Progress: 1.043
Progress: 1.000
Progress: 1.308
Progress: 1.000
Progress: 1.303
Progress: 1.000
Progress: 1.090
Progress: 1.000
Progress: 1.327
Progress: 1.000
Progress: 1.332
Progress: 1.000
Progress: 1.303
Progress: 1.000
Progress: 0.943
Progress: 1.000
Progress: 1.318
Progress: 1.000
Progress: 1.275
Progress: 1.000
Progress: 1.389
Progress: 1.000
Progress: 1.351
Progress: 1.000
Progress: 1.192
Progress: 1.000
Progress: 1.357
Progress: 1.000
Progress: 1.398
Progress: 1.000
Progress: 1.313
Progress: 1.000
Progress: 1.370
Progress: 1.000
After
Progress: 0.018
Progress: 0.027
Progress: 0.044
Progress: 0.045
Progress: 0.062
Progress: 0.077
Progress: 0.095
Progress: 0.112
Progress: 0.130
Progress: 0.147
Progress: 0.147
Progress: 0.165
Progress: 0.182
Progress: 0.199
Progress: 0.216
Progress: 0.216
Progress: 0.225
Progress: 0.228
Progress: 0.245
Progress: 0.246
Progress: 0.263
Progress: 0.280
Progress: 0.281
Progress: 0.288
Progress: 0.289
Progress: 0.306
Progress: 0.320
Progress: 0.321
Progress: 0.338
Progress: 0.338
Progress: 0.356
Progress: 0.356
Progress: 0.374
Progress: 0.387
Progress: 0.405
Progress: 0.422
Progress: 0.426
Progress: 0.444
Progress: 0.461
Progress: 0.479
Progress: 0.496
Progress: 0.496
Progress: 0.514
Progress: 0.532
Progress: 0.549
Progress: 0.549
Progress: 0.566
Progress: 0.567
Progress: 0.584
Progress: 0.584
Progress: 0.602
Progress: 0.619
Progress: 0.619
Progress: 0.637
Progress: 0.654
Progress: 0.671
Progress: 0.672
Progress: 0.689
Progress: 0.689
Progress: 0.707
Progress: 0.725
Progress: 0.742
Progress: 0.760
Progress: 0.777
Progress: 0.777
Progress: 0.795
Progress: 0.812
Progress: 0.830
Progress: 0.847
Progress: 0.851
Progress: 0.869
Progress: 0.886
Progress: 0.904
Progress: 0.921
Progress: 0.939
Progress: 0.939
Progress: 0.955
Progress: 0.972
Progress: 0.972
Progress: 0.981
Progress: 0.982
Progress: 1.000

@ZachNagengast
Copy link
Contributor

For some reason watchos is timing out, could you try the simulator tests for watchos? I will update the timeout if those succeed locally

Copy link
Contributor

@ZachNagengast ZachNagengast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving pending tests passing

@finnvoor
Copy link
Contributor Author

finnvoor commented Jul 3, 2024

Not having any issues with the tests locally, the same test command passes. Not sure what the issue is with the CI.

@ZachNagengast
Copy link
Contributor

The macos 13 xlarge tests passed so I think this is good to go. Just needs a higher timeout for the smaller runner.

@ZachNagengast
Copy link
Contributor

ZachNagengast commented Jul 3, 2024

Ok the errors are due to the simulator not booting fast enough, will update with a fix in a follow up PR that resolves #100

Thanks for this submission! This will make the progress hard to estimate full time to complete because it will depend on the window size in the VAD chunks, but it is a great workaround in the meantime 🎉

@ZachNagengast ZachNagengast merged commit b3f12fc into argmaxinc:main Jul 3, 2024
5 of 9 checks passed
@ZachNagengast
Copy link
Contributor

ZachNagengast commented Jul 3, 2024

FYI just merged this #180 there seems to be an issue with the combine stuff on watchOS, so I limited the test to not run on that platform. Not sure whats causing it but its only happening on macOS 14 runners with low compute resources.

Ryan0520 pushed a commit to Ryan0520/WhisperKit that referenced this pull request Jul 17, 2024
* commit 'c93d613c8c6fc3ec5b9d1d9da5d1f4206183c5e4':
  Fix language logits filter
  Fix functional tests thread issue
  Fix resampling large files (argmaxinc#183)
  Fix indeterminate tests (argmaxinc#180)
  Fix progress when using VAD chunking (argmaxinc#179)
  Update unit-test.yml timeout

# Conflicts:
#	Sources/WhisperKit/Core/AudioProcessor.swift
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 this pull request may close these issues.

VAD: Progress reporting doesn't report evenly when VAD is active
2 participants