-
Notifications
You must be signed in to change notification settings - Fork 25
Fix Whisper #167
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 Whisper #167
Conversation
0e8e51d to
33ac6d6
Compare
|
Note - ignore the CI test failures, they are unrelated. If you click into the logs you can see that tests are passing until it runs out of disk space, additionally I've verified these test locally as well. |
6c56bd6 to
9e94cef
Compare
|
With this change ASR models are now exported as a single model.pte with encoder and decoder methods instead of a forward correct? Is there a way to still export them as separate encoder/decoder models so that it works with the current version of react-native-executorch? |
|
@dylanschoenmakers yes that's correct, reason is that we usually recommend the one model / multiple method approach since there's less overhead. cc @chmjkb is this something you'd be able to update for react-native-excecutorch? |
|
@jackzhxng, sure we'll update it as soon as we can, thanks for fixing this! Any ideas on what the perf improvements should be? |
I'm getting 3.4 tok/s on whisper-small (not tiny) |
I think having both methods in one file makes sure that they can work together. We also recognize it makes the model artifact much bigger, but I believe using program data separation can mitigate this issue. |
Oh if you are curious about perf improvement between separate files and one file, I don’t think there will be significant perf improvement, but just the benefit I commented earlier. |
Fixes Whisper export + misc
seq2seq-related cleanups, which was broken a while back due to transformers changesRuns on both portable and XNNPack.
Stack from (oldest at bottom):