Skip to content

Conversation

@chiral-carbon
Copy link
Contributor

What does this PR do?

Fixes #6891 for Advanced Diffusion Training SDXL with LoRA

Notebook example: Jupyter notebook
Example model card: Model Card

Before submitting

Who can review?

@sayakpaul @linoytsaban

@sayakpaul sayakpaul requested a review from linoytsaban April 10, 2024 03:15
Copy link
Collaborator

@linoytsaban linoytsaban left a comment

Choose a reason for hiding this comment

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

Hey @chiral-carbon! same small comment from the sd15 PR, applies here too - since it can be either a LoRA or a DoRA, we need to check if the use_dora arg is True and adapt the tag accordingly :)
other than that, all good!

@chiral-carbon
Copy link
Contributor Author

thanks @linoytsaban! have addressed the use_dora check, please review!

@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.

"stable-diffusion-xl-diffusers",
"text-to-image",
"diffusers",
"lora",
Copy link
Collaborator

Choose a reason for hiding this comment

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

last thing 😊- we need to switch "lora" -> lora in the tags here (to use the var defined in line 114)

@linoytsaban
Copy link
Collaborator

Hey @chiral-carbon! I think once we address the "lora" -> lora change it'll also pass all the tests and we can merge :)

@chiral-carbon
Copy link
Contributor Author

Hi @linoytsaban my apologies, I completely missed this message, will do it asap!

@chiral-carbon
Copy link
Contributor Author

@linoytsaban pushed it!

@linoytsaban
Copy link
Collaborator

Thanks @chiral-carbon! I'm not sure what's failing the quality tests now 🧐 could you run make fix up please?

@chiral-carbon
Copy link
Contributor Author

hi @linoytsaban this is the output of make fixup:

(hf-diff) ada@Abhipshas-MacBook-Pro diffusers % make fixup
Checking/fixing examples/advanced_diffusion_training/train_dreambooth_lora_sd15_advanced.py
Found 3 errors (3 fixed, 0 remaining).
1 file reformatted
python utils/custom_init_isort.py
python utils/check_doc_toc.py --fix_and_overwrite
running deps_table_update
updating src/diffusers/dependency_versions_table.py
python utils/check_dummies.py
python utils/check_repo.py
Traceback (most recent call last):
  File "/Users/ada/Desktop/Projects/diffusers/utils/check_repo.py", line 25, in <module>
    from diffusers.models.auto import get_values
ModuleNotFoundError: No module named 'diffusers.models.auto'
make: *** [repo-consistency] Error 1

it seems that it has no module named models.auto?

@tolgacangoz
Copy link
Contributor

Could you run make style && make quality?

@chiral-carbon
Copy link
Contributor Author

this is the output of make style && make quality:

(hf-diff) ada@Abhipshas-MacBook-Pro diffusers % make style && make quality
ruff check examples scripts src tests utils benchmarks setup.py --fix
ruff format examples scripts src tests utils benchmarks setup.py
1 file reformatted, 874 files left unchanged
doc-builder style src/diffusers docs/source --max_len 119
/Library/Developer/CommandLineTools/usr/bin/make autogenerate_code
running deps_table_update
updating src/diffusers/dependency_versions_table.py
/Library/Developer/CommandLineTools/usr/bin/make extra_style_checks
python utils/custom_init_isort.py
python utils/check_doc_toc.py --fix_and_overwrite
ruff check examples scripts src tests utils benchmarks setup.py
ruff format --check examples scripts src tests utils benchmarks setup.py
875 files left unchanged
doc-builder style src/diffusers docs/source --max_len 119 --check_only
python utils/check_doc_toc.py

have pushed it as well, it changed the formatting of the widget_dict append operation in line 124

@chiral-carbon
Copy link
Contributor Author

should I also run make test? the contributor guidelines said that it would take time to run normally so I skipped it

@chiral-carbon
Copy link
Contributor Author

hi @linoytsaban @tolgacangoz! is this PR good to be merged?

@tolgacangoz
Copy link
Contributor

tolgacangoz commented Jul 14, 2024

It seems that there are conflicts that must be resolved. Could you resolve them?

@chiral-carbon
Copy link
Contributor Author

@tolgacangoz done

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

@github-actions github-actions bot added the stale Issues that haven't received updates label Sep 14, 2024
@a-r-r-o-w a-r-r-o-w removed the stale Issues that haven't received updates label Oct 17, 2024
@a-r-r-o-w a-r-r-o-w requested a review from linoytsaban October 17, 2024 20:43
@chiral-carbon
Copy link
Contributor Author

@a-r-r-o-w hi! Is this and the PR #7613 good to be merged?

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

@github-actions github-actions bot added the stale Issues that haven't received updates label Nov 25, 2024
@sayakpaul
Copy link
Member

@linoytsaban a gentle ping here

@github-actions github-actions bot removed the stale Issues that haven't received updates label Nov 26, 2024
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

@github-actions github-actions bot added the stale Issues that haven't received updates label Dec 20, 2024
@chiral-carbon
Copy link
Contributor Author

@sayakpaul @a-r-r-o-w @linoytsaban should I close this PR unless it is good to be merged? has been stale for a while

@a-r-r-o-w
Copy link
Contributor

Sorry for the delay. I'm triggering the tests again to make sure they pass and will merge once they do

@a-r-r-o-w a-r-r-o-w merged commit d9023a6 into huggingface:main Apr 3, 2025
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale Issues that haven't received updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Tracker] use the new model card utilities for saving model cards from the training script

6 participants