ModularPipeline.save_pretrained: change the default overwrite_modular_index to be True - #14659
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
|
@yiyixuxu Sounds good to me. Did you also want to overwrite for cases where pipe = ModularPipeline.from_pretrained(repo_id, local_dir=<path to dir>)
pipe.load_components() |
|
@DN6
|
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HCdbvRpL9fv3h3WwSUPpfS
Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com>
Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com>
Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com>
Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com>
| component_spec_dict["pretrained_model_name_or_path"] = repo_id if push_to_hub else save_directory | ||
| component_spec_dict["subfolder"] = component_name | ||
| component_spec_dict["variant"] = variant if save_method_accept_variant else None | ||
| component_spec_dict["revision"] = None |
There was a problem hiding this comment.
But is there any reason to include revision here?
There was a problem hiding this comment.
just in case the original entry on modular_model_index has a revision and we overwrite, we need to set it to None since it would not apply for the "local compnent"
| with open(os.path.join(save_dir, "modular_model_index.json")) as f: | ||
| index = json.load(f) | ||
|
|
||
| _library, _cls, unet_spec = index["unet"] | ||
| assert unet_spec["pretrained_model_name_or_path"] == save_dir | ||
| assert unet_spec["subfolder"] == "unet" | ||
|
|
||
| _library, _cls, vae_spec = index["vae"] | ||
| expected_vae = save_dir if overwrite_modular_index else "hf-internal-testing/tiny-stable-diffusion-xl-pipe" | ||
| assert vae_spec["pretrained_model_name_or_path"] == expected_vae |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HCdbvRpL9fv3h3WwSUPpfS
|
@yiyixuxu Just a follow up to this. If the intention is to download/load to a pipe = ModularPipeline.from_pretrained(repo_id)
pipe.load_components() # in the HF cache
pipe.save_pretrained(local_dir) # in the local dir |
|
That is intentional, I think? If the user doesn't prefer the default path or something like that? |
|
@sayakpaul @DN6 |
|
@yiyixuxu With pipe = DiffusionPipeline.from_pretrained(repo_id, local_dir=<my local dir>) # saved to local dir and not HF cache
pipe_local = DiffusionPipeline.from_pretrained(<my local dir>) # nothing is downloaded. Loads straight from local pathWith modular's two step loading, we always end up with weights in the HF cache. pipe = ModularPipeline.from_pretrained(repo_id)
pipe.load_components() # weights in the HF cache
pipe.save_pretrained(local_dir) # weights in the local dirSo the user will always have to clear their cache after saving the model to the local dir. I'm proposing we introduce an option that allows them to avoid having to do that. |
|
@DN6 |
cc @DN6
Just a proposal, but I think since
save_pretrainednow saves the loaded components by default, it makes sense foroverwrite_modular_indexto default toTrueas well — otherwise the result is not very intuitive: e.g. when you runthe components are all saved into
local_dir, but itsmodular_model_index.jsonstill points atrepo_id— so reloading from the local_dir still fetch everything from hub even though weights are in the local folderIt's especially confusing if you intended to save a self-contained local copy with some custom components
See related issues: #14251, #14640