-
-
Notifications
You must be signed in to change notification settings - Fork 166
Simplify and fix --save_merged_model #594
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
merge_lora_weights is called not only by Wan but also by Flux Kontext, FramePack, and Qwen-Image. Is it possible to fix and simplify the issue by including those? If not, please fix only Wan and I will merge this into a new branch and make additional fixes.
| f.write(hjson) | ||
|
|
||
| for k, v in tensors.items(): | ||
| for k, v in tqdm(tensors.items(), desc=f"Saving model to {filename}...", total=len(tensors)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is called not only for saving models but also for saving caches, so the logging output by tqdm is too verbose. Could you please remove this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah woops, yes can do!
I can also make the progress bar an option on mem_eff_save_file.
What do you prefer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! I think anyone who uses this option understands that saving after merging takes some time, so I think it's fine to simply delete tqdm.
I'll take a look and see if I can centralize the fix there instead |
f811f89 to
4fcc570
Compare
|
Hi @kohya-ss, I've given a shot at refactoring the other models as well. It's not as simple as only overriding I've made the shortcut model saving + exits in both of those functions and tried to link it up in each of the generation scripts. I don't have loras to try out all of these different models now, so please give this an extra good read over! |
|
Oh and I removed the progress bar from |
|
Thank you for update! Could you please format it in ruff? Also, please understand that I may make some changes after merging. |
|
Of course, any changes that you see fit! |
--save_merged_model was not saving anything for me, just loading the model in and exiting.
Based on my reading, I think only lycoris-based merges get saved with the current logic.
What I've done to address this:
load_dit_modelso that saving works the same regardless of which lora style orprocess_*function is usedsys.exit(0)rather than returning and relying on all calling code to exit correctly as wellmem_eff_save_filefor some more visual feedback