Add convenience methods to detect if a model is installed #236
Conversation
|
Thanks for adding this @bmurray looks useful! Can you think of a test to add to verify it is working? Can be part of the unit tests here: https://github.com/argmaxinc/WhisperKit/blob/main/Tests/WhisperKitTests/UnitTests.swift Also, was the need for this derived from needing offline access to the models? Or some other motivation? We definitely want to make sure offline access is always available so if there was a regression there from a recent release we should fix that. |
|
Yea I'm working on replacing WhisperCPP in my app with this, and this is a feature that I needed. It's more or less just detecting if the model is already installed or not, so I can provide feedback to users. I'm probably going to add some other additions here in a bit that provide other feedback, such as if the model is downloading. As far as I can tell, theres no visibility into whats happening if it "auto" downloads a model when a transcription starts. I'll take a look to see if theres a unit test that makes sense. |
…lready downloaded, without querying online servers.
|
I took a look and I'm not sure where to put any tests for this that don't require downloading models just to test if they're installed. I updated the pull to do offline support. With download: false set, and the model already downloaded (either manually or automatically), the transcriber can run. It probably needs some more testing as theres a bunch of things that silently query websites. |
|
Ok cool, I think a good test would start in a similar way to this one, where it just loads the model (which would imply its downloaded), then simply make sure the new functions correctly detect that it is installed. Also I'd recommend keeping this interface the same |
|
Yea it makes sense not to break the API. I figured it was just unifying the API but it could break it for others. I added a couple tests to ensure it works. Seems to be testing on my end here. |
| @@ -266,12 +266,87 @@ open class WhisperKit { | |||
| throw error | |||
| } | |||
| } | |||
| public static func modelLocation( | |||
There was a problem hiding this comment.
Should add a new line above this for code style. Will want to add docc for all of the functions in this file eventually but not a blocker.
| throw error | ||
| } | ||
| } | ||
| public static func modelInstalled( |
There was a problem hiding this comment.
New line above this as well
| "Model was not installed" | ||
| ) | ||
| XCTAssertTrue( | ||
| WhisperKit.modelInstalled(variant: "tiny.en"), |
There was a problem hiding this comment.
You'll need some method to download this one ahead of time before checking it is installed.
you can do that by either adding a linemake download-model MODEL=tiny.en here to download it, or just removing this tiny.en check and only running this test for tiny.
| let location2 = try WhisperKit.modelLocation(variant: "openai_whisper-tiny") | ||
| XCTAssertEqual(location, location2, "Auto fallback to OpenAI model returned a different result") | ||
| } | ||
| func testOfflineMode() async throws { |
There was a problem hiding this comment.
New line above this one
| .withPrevious() | ||
| .sink { previous, current in | ||
| if let previous { | ||
| XCTAssertLessThan(previous, current) |
This adds convenience functions to detect if a model is installed locally, without querying the server.