Merged
Conversation
When `cactus convert` merges a LoRA adapter, it saves the merged model to a temp directory and tries to “download” it as if it were a Hugging Face repo id. The fix checks if the model_id is a local directory and passes the temp path directly into weight conversion by skipping the HF path Signed-off-by: Lennart <[email protected]>
- copy sentence piece model from LoRA directory (or base model in the HF cache as a fallback) and read the tokenizer config locally if possible - the model_id is a path in the merging case, which poisend the whole conversion chain -> replaced with a name variable for name-based detection (e.g. gemma special tokens) Signed-off-by: Lennart <[email protected]>
Signed-off-by: Lennart <[email protected]>
Contributor
Author
|
I added one more fix, so that the tokenizer config is also copied from the LoRA directory if present |
Collaborator
|
thanks for this @lennartvoelz !!! |
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
2 participants
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.
This merge request fixes issue #490
The cactus convert pipeline broke in two related ways when working with merged LoRA models:
Tokenizer detection failed: The tokenizer lookup relied on model_id being a Hugging Face repo identifier (e.g. google/functiongemma-270m-it). When merging a LoRA adapter, model_id is set to a local filesystem path, which poisoned name-based detection logic (e.g. special token handling for Gemma models) and caused the sentence piece model lookup to fail entirely.
Merged model treated as HF repo: After merging, the merged weights are written to a temporary directory. The conversion pipeline then attempted to "download" this temp directory as if it were a remote Hugging Face repo ID.
Fix
Tokenizer (fix: tokenizer detection failed for LoRA merged models)
LoRA merge path handling (fix: treat merged LoRA temp dir as local model path)
Impact of the fixes
Before:
After:
-> The output also shows the special gemma tokens now
I ran the test suite and everything works fine