Uh oh!
There was an error while loading. Please reload this page.
[modular] not pass trust_remote_code to external repos - #13204
Conversation
sayakpaul
left a comment
There was a problem hiding this comment.
I like it!
I left some thoughts to discuss. LMK if anything is unclear.
| f"To load this component manually:\n" | ||
| f" from diffusers import AutoModel\n" | ||
| f' {name} = AutoModel.from_pretrained("{spec.pretrained_model_name_or_path}", trust_remote_code=True)\n' | ||
| f" pipe.update_components({name} = {name})\n" | ||
| ) | ||
| logger.warning(warning_msg) |
There was a problem hiding this comment.
Of course, we cannot generalize to all possible combinations but what if the underlying component is a scheduler or a non-modeling component?
For schedulers, we could have AutoScheduler or something so that users won't have to remember a specific class name. For others (such as tokenizers, image processors), we already have "auto" classes.
I guess it's not that big a use case yet, but just wanted to flag.
There was a problem hiding this comment.
Also, should we error instead? If the user-side logger is not configured appropriately then this warning could go unnoticed.
There was a problem hiding this comment.
i updated the message
tried to make it error but reverted back to warning for now: it would be a little annoying that you won't be able to use load_components() because one element is not loadable
| block = ModularPipelineBlocks.from_pretrained(pipeline_repo_dir, trust_remote_code=True) | ||
| pipe = block.init_pipeline() | ||
| pipe.save_pretrained(pipeline_repo_dir) |
There was a problem hiding this comment.
Should we also inspect the modular_model_index.json file here (checking if the repo paths were serialized as expected)?
There was a problem hiding this comment.
I think if we can load it back, we are fine (we loaded it back next and verified the path too)
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Dhruv Nair <dhruv.nair@gmail.com>
| assert pipe.unet is not None | ||
| assert getattr(pipe, "vae", None) is None | ||
| def test_load_components_selective_loading_incremental(self): |
There was a problem hiding this comment.
Do we also test if pipe doesn't have other expected components (like scheduler)?
| f"to the pipeline via `pipe.update_components()`. For example, if it is a custom model:\n\n" | ||
| f' {name} = AutoModel.from_pretrained("{spec.pretrained_model_name_or_path}", trust_remote_code=True)\n' | ||
| f" pipe.update_components({name}={name})\n" | ||
| ) |
| import json | ||
| from diffusers import UNet2DConditionModel |
There was a problem hiding this comment.
(nit): maybe we could shift all the imports to the top.
| for key in original_state_dict: | ||
| assert torch.equal(original_state_dict[key], loaded_state_dict[key]), f"Mismatch in {key}" | ||
| def test_save_pretrained_updates_index_for_model_with_no_load_id(self, tmp_path): |
Uh oh!
There was an error while loading. Please reload this page.
…3204) * add * update warn * add a test * updaqte * update_component with custom model * add more tests * Apply suggestion from @DN6 Co-authored-by: Dhruv Nair <dhruv.nair@gmail.com> * up --------- Co-authored-by: yiyi@huggingface.co <yiyi@ip-26-0-161-123.ec2.internal> Co-authored-by: Dhruv Nair <dhruv.nair@gmail.com>
based on this discussion
huggingface/blog#3278 (comment)