fix: read flm_version directly when checking compatibility (#545) - #571
Open
istitov wants to merge 1 commit into
Open
fix: read flm_version directly when checking compatibility (#545)#571istitov wants to merge 1 commit into
istitov wants to merge 1 commit into
Conversation
check_model_compatibility() built a base LM_Config and called from_pretrained() only to read flm_version, which runs the decoder-LM shape asserts (hidden_size > 0, ...). A HuggingFace Whisper config.json carries d_model rather than hidden_size, so the assert aborts `flm list` and `flm serve --asr 1`. The linux-default Release build sets -DNDEBUG, making assert() a no-op, so the crash is masked there. A compatibility check only needs flm_version, so read it directly from config.json: model-type agnostic and independent of LM-shape fields.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #545.
Problem
After
flm pull whisper-v3:turbo, both flm list and flm serve --asr 1 abort:Assertion
this->hidden_size > 0failed. (lm_config.hpp:121)Root cause
ModelDownloader::check_model_compatibility() reads each model's flm_version by constructing a base LM_Config and calling from_pretrained() — which
runs the decoder-LM shape asserts (hidden_size > 0, …). A HuggingFace Whisper config.json carries d_model, not hidden_size, so hidden_size stays
0 and the assert fires. Whisper_Config exists and reads d_model, but from_pretrained is non-virtual and this scan path uses the base class for
every model regardless of model_type.
Why it doesn't reproduce on the default build: the linux-default preset is Release (-DNDEBUG), so assert() is a no-op and the path runs silently.
Builds with asserts live abort.
Fix
A compatibility check only needs flm_version, so read it straight from config.json instead of constructing + validating a full LM config.
Why this approach
from LM-loadability validation.
Whisper.
A broader fix (make from_pretrained virtual + dispatch by model_type at all scan sites) is left out of scope. The Whisper loading path already
uses Whisper_Config and is untouched.
Testing
Built with asserts live (no -DNDEBUG) against a pristine whisper-v3:turbo config: flm list completes (whisper-v3:turbo ✅) and
flm serve --asr 1loads the model and starts the server. Both aborted before.