search

argmaxinc / WhisperKit

On-device Speech Recognition for Apple Silicon
http://argmaxinc.com/blog/whisperkit
MIT License
3.92k stars 331 forks source link

skip functional tests for models that are not downloaded. #48

Closed metropol closed 8 months ago

metropol commented 8 months ago

hi all, it seems that functional tests require two models to be downloaded: openai_whisper-tiny (because of testRealTimeFactorTiny()) and openai_whisper-large-v3 (because of testInitLarge() and testRealTimeFactorLarge()). This should probably be added to the CONTRIBUTING.md, however this PR does not make this change.

Another functional test, testOutputAll() - actually allModelPaths(), assumes that folders in ./Models/whisperkit-coreml, which are created from the make setup-model-repo, contain actual models, while in fact only models explicitly downloaded with make download-model MODEL=x or make download-models are present.

testOutputAll() fails if not all models have been downloaded, which is not ideal. Running all tests for all models (takes a long time and) makes sense in some scenarios, but probably not to test some code changes.

I've implemented some changes so that allModelPaths() only returns folders that contain downloaded models. This is done by checking if a proxy file (MelSpectrogram.mlmodelc/coremldata.bin) is a git lfs pointer file (starting with version https://…). If it is, then the folder is not included.

metropol commented 8 months ago

Moved the typo fix to its own branch (for a future PR).

metropol commented 8 months ago

It failed when building for VisionOS, which I don't have as a platform locally. For the typo fix (langauges -> languages), as you prefer: I can add it back to this branch or create another PR just for it.

ZachNagengast commented 8 months ago

@metropol whatever is more convenient for you, probably just reverting the commit that removes it would be simplest.

metropol commented 8 months ago

@ZachNagengast I've reverted the commit the removes the typo.