fix: disable LoRA export again#319
Conversation
This reverts commit da92f74.
![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 modifies the obtain_merge_strategy function to replace the adapter-only save option with a cancel option and introduces conditional logic for the merge prompt. A review comment points out that nesting the prompt within a specific conditional block makes certain quantization checks redundant and prevents users not using 4-bit quantization from accessing the cancel option. A suggestion was provided to move the prompt selection outside the conditional block to restore full functionality and clean up the logic.
| strategy = prompt_select( | ||
| "How do you want to proceed?", | ||
| choices=[ | ||
| Choice( | ||
| title="Merge LoRA into full model" | ||
| + ( | ||
| "" | ||
| if settings.quantization == QuantizationMethod.NONE | ||
| else " (requires sufficient RAM)" | ||
| ), | ||
| value="merge", | ||
| ), | ||
| value="merge", | ||
| ), | ||
| Choice( | ||
| title="Save LoRA adapter only (can be merged later)", | ||
| value="adapter", | ||
| ), | ||
| ], | ||
| ) | ||
| Choice( | ||
| title="Cancel", | ||
| value="cancel", | ||
| ), | ||
| ], | ||
| ) | ||
|
|
||
| if strategy == "cancel": | ||
| return None | ||
|
|
||
| return strategy | ||
| return strategy | ||
| else: | ||
| return "merge" |
There was a problem hiding this comment.
Moving the prompt outside the if block restores the ability to cancel the operation for all users (not just those using 4-bit quantization) and makes the conditional title logic meaningful again. In the current implementation, the check settings.quantization == QuantizationMethod.NONE at line 141 is redundant because it is nested inside a block that only executes when quantization is BNB_4BIT.
strategy = prompt_select(
"How do you want to proceed?",
choices=[
Choice(
title="Merge LoRA into full model"
+ (
""
if settings.quantization == QuantizationMethod.NONE
else " (requires sufficient RAM)"
),
value="merge",
),
Choice(
title="Cancel",
value="cancel",
),
],
)
if strategy == "cancel":
return None
return strategy
* fix: Reset model after saving merged model The adapter is lost and writes 0-byte adapters if you save an adapter after saving the merged model. * Revert "Revert "Revert "fix: disable LoRA export for now" (#308)" (#319)" This reverts commit 216c089. * Add comment as to why resetting model is needed
1 participant
Reverts #308