fix: Reset model after saving merged model#321
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the model resetting and abliteration logic into a local function named reset_trial_model within count_completed_trials. This function is now called in the original execution flow as well as after saving the model and pushing it to the Hugging Face Hub. A review comment pointed out that the new function signature lacks a return type annotation, which is a violation of the repository's style guide.
|
Yeah, now that I figured out what the problem is, I was actually able to find a discussion confirming just that: huggingface/peft#868 (comment) Not having an option to disable this behavior is very poor API design. |
| del merged_model | ||
| empty_cache() | ||
| model.tokenizer.save_pretrained(save_directory) | ||
| reset_trial_model() |
There was a problem hiding this comment.
Maybe add a comment here explaining why you are doing this.
|
The fix is correct, but the PR is missing the logic from #308, since that was reverted again. |
The adapter is lost and writes 0-byte adapters if you save an adapter after saving the merged model.
Embedded revert-revert in this PR. |
|
Thanks, merged! Feels good to finally understand what was going wrong before. |
2 participants
The adapter is lost and writes 0-byte adapters if you save an adapter after saving the merged model.