Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[Refactor] Update from single file #6428

Merged
merged 94 commits into from
Jan 23, 2024
Merged

[Refactor] Update from single file #6428

merged 94 commits into from
Jan 23, 2024

Conversation

DN6
Copy link
Collaborator

@DN6 DN6 commented Jan 2, 2024

What does this PR do?

from_single_file relies on download_from_original_stable_diffusion_ckpt which is now quite difficult to read and has a number of subtle issues.

This PR:

  • Refactors from single file so that it now builds the individual components of the invoking pipeline class and creates the pipeline. This ensures that the correct pipeline class is always created when using from single file.
  • Removes circular dependencies in the original single file mixin by removing imports of the actual pipeline objects
  • Removes multiple if-else blocks in favour of using dedicated functions to download the relevant configs and extract parameters from checkpoints.
  • Moves FromOriginalVAEMixin and FromOriginalControlnetMixin into their own modules.

Some Notes:

  • Tests for the single file mixin rely on using real checkpoints with test_download_ckpt_diff_format_is_same. As such, only slow tests have been added.

Fixes # (issue)

Before submitting

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@@ -389,7 +389,12 @@ def _get_pipeline_class(
return class_obj

diffusers_module = importlib.import_module(class_obj.__module__.split(".")[0])
class_name = config["_class_name"]
class_name = class_name or config["_class_name"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that fully backwards compatible?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I searched the code base and it seems like class_name or config is always passed here.

@patrickvonplaten
Copy link
Contributor

Nice PR - things look more or less good to go for me. Some things I'd like to resolve before merging:

  • 1.) Do we want to re-use the same "single-file" loading class for both models and pipelines (see comment here). Could it maybe be better to write a shorter FromSingleModelFileMixin class for models?
  • 2.) When changing code in pipeline_utils.py we need to be extra careful. Let's make sure to monitor all slow tests once this PR is merged
  • 3.) We should add some fast tests here as well. Can you take some of our diffusers-format tiny checkpoints: https://huggingface.co/hf-internal-testing/tiny-stable-diffusion-pipe

@HuggingFaceDocBuilderDev

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.

Copy link
Contributor

@patrickvonplaten patrickvonplaten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR is good to merge for me to get going here. Let's make sure we also refactor the "model-single-file" loaders in a follow-up PR.

@DN6
Copy link
Collaborator Author

DN6 commented Jan 22, 2024

I checked and the the slow tests are passing. I can add the model single-file refactor and fast tests in a follow up.

):
vae_scaling_factor = original_config["model"]["params"]["scale_factor"]
else:
vae_scaling_factor = 0.18215 # default SD scaling factor
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If no scaling factor is provided, the default should be set to 0.18215

AmericanPresidentJimmyCarter pushed a commit to AmericanPresidentJimmyCarter/diffusers that referenced this pull request Apr 26, 2024
* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update'

* update

* update

* update

* update

* update

* update

* up

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* up

* update

* update

* update

* update

* update'

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* clean

* update

* update

* clean up

* clean up

* update

* clean

* clean

* update

* updaet

* clean up

* fix docs

* update

* update

* Revert "update"

This reverts commit dbfb8f1.

* update

* update

* update

* update

* fix controlnet

* fix scheduler

* fix controlnet tests
@sayakpaul sayakpaul deleted the refactor-single-file branch December 3, 2024 10:24
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants