fix VAE loading issue in train_dreambooth #7632

bssrdf · 2024-04-10T13:04:04Z

What does this PR do?

This PR is similar to #3462 but fixes the same issue in train_dreambooth.py.

Fixes #7619

Before submitting

This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
Did you read the contributor guideline?
Did you read our philosophy doc (important for complex PRs)?
Was this discussed/approved via a GitHub issue or the forum? Please add a link to it if that's the case.
Did you make sure to update the documentation with your changes? Here are the
documentation guidelines, and
here are tips on formatting docstrings.
Did you write any new necessary tests?

Who can review?

@sayakpaul @standardAI

sayakpaul · 2024-04-11T03:26:24Z

We need to make sure the code quality tests pass.

HuggingFaceDocBuilderDev · 2024-04-11T03:33:16Z

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.

tolgacangoz · 2024-04-11T06:11:02Z

@bssrdf could you run make style && make quality?

bssrdf · 2024-04-11T13:22:50Z

@bssrdf could you run make style && make quality?

Done!

sayakpaul · 2024-04-12T03:03:02Z

It still doesn't pass :/

bssrdf · 2024-04-12T12:21:17Z

It still doesn't pass :/

I don't know what's going on. Any way, I rerun make style and make quality and here are the outputs. Nothing stands out.

$ make style
ruff check examples scripts src tests utils benchmarks setup.py --fix
warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `pyproject.toml`:
  - 'ignore' -> 'lint.ignore'
  - 'select' -> 'lint.select'
  - 'isort' -> 'lint.isort'
  - 'per-file-ignores' -> 'lint.per-file-ignores'
All checks passed!
ruff format examples scripts src tests utils benchmarks setup.py
warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `pyproject.toml`:
  - 'ignore' -> 'lint.ignore'
  - 'select' -> 'lint.select'
  - 'isort' -> 'lint.isort'
  - 'per-file-ignores' -> 'lint.per-file-ignores'
876 files left unchanged
doc-builder style src/diffusers docs/source --max_len 119
make autogenerate_code
make[1]: Entering directory '/home/isodden/temp/diffusers'
running deps_table_update
updating src/diffusers/dependency_versions_table.py
make[1]: Leaving directory '/home/isodden/temp/diffusers'
make extra_style_checks
make[1]: Entering directory '/home/isodden/temp/diffusers'
python utils/custom_init_isort.py
python utils/check_doc_toc.py --fix_and_overwrite
make[1]: Leaving directory '/home/isodden/temp/diffusers'

$make quality
ruff check examples scripts src tests utils benchmarks setup.py
warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `pyproject.toml`:
  - 'ignore' -> 'lint.ignore'
  - 'select' -> 'lint.select'
  - 'isort' -> 'lint.isort'
  - 'per-file-ignores' -> 'lint.per-file-ignores'
All checks passed!
ruff format --check examples scripts src tests utils benchmarks setup.py
warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `pyproject.toml`:
  - 'ignore' -> 'lint.ignore'
  - 'select' -> 'lint.select'
  - 'isort' -> 'lint.isort'
  - 'per-file-ignores' -> 'lint.per-file-ignores'
876 files already formatted
doc-builder style src/diffusers docs/source --max_len 119 --check_only
python utils/check_doc_toc.py

$git log
commit ee9b19e53e4ff1b14a001fadc140f19c47959bb1 (HEAD -> fix-train-dreambooth-vae-not-loading, origin/fix-train-dreambooth-vae-not-loading)
Merge: ac68fe51 279de3c3
Author: Sayak Paul <spsayakpaul@gmail.com>
Date:   Fri Apr 12 08:33:05 2024 +0530

    Merge branch 'main' into fix-train-dreambooth-vae-not-loading

commit 279de3c3ffedcb1394518a8f1c950fa30f272390
Author: Sai-Suraj-27 <sai.suraj.27.729@gmail.com>
Date:   Fri Apr 12 01:13:01 2024 +0530

    fix: Replaced deprecated `logger.warn` with `logger.warning` (#7643)

    Fixed deprecated logger.warn with logger.warning.

commit 8e14535708f6af0794148150f5c073c4723dbbae
Author: Yiqin Zhao <yiqinzhao@outlook.com>
Date:   Thu Apr 11 15:08:42 2024 -0400

    Fixed YAML loading. (#7579)

commit 0bee4d336b925b6064eee156f5a155e3ca3b30ab
Author: dg845 <58458699+dg845@users.noreply.github.com>
Date:   Thu Apr 11 10:52:12 2024 -0700

    LCM Distill Scripts Fix Bug when Initializing Target U-Net (#6848)

    * Initialize target_unet from unet rather than teacher_unet so that we correctly add time_embedding.cond_proj if necessary.

    * Use UNet2DConditionModel.from_config to initialize target_unet from unet's config.

    ---------

    Co-authored-by: Sayak Paul <spsayakpaul@gmail.com>

commit ac68fe5151a7e1bba83167bd07b65aded716174b
Author: bssrdf <bssrdf@gmail.com>
Date:   Thu Apr 11 09:22:17 2024 -0400

    reformat code with make style && make quality

tolgacangoz · 2024-04-12T12:29:30Z

Could you reverse the last changes make style && make quality done, and downgrade your ruff, then run the command again? Actually, install diffusers from the source:

python -m uv pip install -e ".[dev]"

Maybe ruff in the diffusers should be upgraded, Idk 🤔.

bssrdf · 2024-04-12T12:53:02Z

I downgraded ruff and reran the "make style && make quality". Let's give it another push. Yeah, ruff is now on 0.3.5. diffusers uses 0.1.5 which is pretty old.

tolgacangoz

It seems OK now. Thanks for fixing this!

bghira · 2024-04-25T13:46:58Z

examples/dreambooth/train_dreambooth.py

+    try:
        vae = AutoencoderKL.from_pretrained(
            args.pretrained_model_name_or_path, subfolder="vae", revision=args.revision, variant=args.variant
        )
-    else:
+    except OSError:
+        # IF does not have a VAE so let's just set it to None
+        # We don't have to error out here
        vae = None


we can't blindly just set None for the VAE if it fails to load. assuming it's DeepFloyd isn't great, the reason that check was appropriate before is because model_has_vae would check for an actual VAE config in the repo.

that check model_has_vae is broken, hence this PR and #3462. I am not the first one who encountered this problem. This PR will make train_dreambooth work for users until the check is fixed.

the OSError version of this check isn't going to work. the model loader throws OSError for other reasons:

raise OSError( "You seem to have cloned a repository without having git-lfs installed. Please install " "git-lfs and run `git lfs install` followed by `git lfs pull` in the folder " "you cloned." )

I figured out the problem, which is specific only on Windows OS. Please review the updated PR with model_has_vae check augmented.

thanks for being patient with me and looking into that deeper. that is a good fix to find.

bghira · 2024-04-25T13:48:46Z

i understand that the other script does this, but that change is from May, 2023. if anything we should fix the method used by this script, and enhance the other scripts to make use of the correct method.

… windows os to make match work

bghira · 2024-04-28T11:04:04Z

examples/dreambooth/train_dreambooth.py

+def model_has_vae(args):
+    config_file_name = os.path.join("vae", AutoencoderKL.config_name)
+    if platform.system() == "Windows":
+        config_file_name = config_file_name.replace('\\', '/')


ohhhhhhh!!! i never use Windows, so this explains a lot.

the os.path.join is actually something we can remove then! it can be f"vae/{AutoencoderKL.config_name}" on line 761 and then the Windows check can go away.

I guess the Git info response will have Linux-like paths in it always.

I removed os.path.join for config_file_name and used '/' for concatenation. It is OS independent now. Thanks for suggestions.

bghira · 2024-04-28T11:04:47Z

examples/dreambooth/train_dreambooth.py

+    if os.path.isdir(args.pretrained_model_name_or_path):
+        config_file_name = os.path.join(args.pretrained_model_name_or_path, config_file_name)
+        return os.path.isfile(config_file_name)


these should remain the same using os.*

These stay the same

sayakpaul

Thanks for finding this. Left a single comment.

sayakpaul · 2024-04-29T02:19:13Z

examples/dreambooth/train_dreambooth.py

 def model_has_vae(args):
-    config_file_name = os.path.join("vae", AutoencoderKL.config_name)
+    config_file_name = f"vae/{AutoencoderKL.config_name}"


Could we make use of config_file_name = Path("vae", AutoencoderKL.config_name).as_posix() here?

@sayakpaul, this will also work. I can change to using Path and as_posix if we all agree it is proper to do.

I think that will be better to promote generality.

i didn't even know that exists. learning things!

Done! Thanks for the tips. I too learned a thing.

yiyixuxu · 2024-05-13T21:27:16Z

@sayakpaul can we merge this now?

sayakpaul · 2024-05-14T02:38:33Z

Sorry for the delay. Once this CI is green, will merge.

fixed vae loading issue huggingface#7619

0ab303b

yiyixuxu requested a review from sayakpaul April 10, 2024 21:37

Merge branch 'main' into fix-train-dreambooth-vae-not-loading

9c8fe84

Merge branch 'main' into fix-train-dreambooth-vae-not-loading

4d4e7bc

bssrdf added 2 commits April 12, 2024 08:41

rerun make style && make quality

8a541a3

Merge branch 'main' into fix-train-dreambooth-vae-not-loading

f04998c

bssrdf force-pushed the fix-train-dreambooth-vae-not-loading branch from ee9b19e to f04998c Compare April 12, 2024 12:49

Merge branch 'main' into fix-train-dreambooth-vae-not-loading

078bdd8

tolgacangoz approved these changes Apr 17, 2024

View reviewed changes

bghira suggested changes Apr 25, 2024

View reviewed changes

bssrdf added 3 commits April 27, 2024 21:58

bring back model_has_vae and add change \ to / in config_file_name on…

80e0acf

… windows os to make match work

add missing import platform

6ad49e2

bring back import model_info

e2e3274

bghira reviewed Apr 28, 2024

View reviewed changes

make config_file_name OS independent

0f8ef79

sayakpaul reviewed Apr 29, 2024

View reviewed changes

sayakpaul and others added 2 commits April 29, 2024 07:49

Merge branch 'main' into fix-train-dreambooth-vae-not-loading

04e9990

switch to using Path.as_posix() to resolve OS dependence

1a9b15e

sayakpaul approved these changes Apr 29, 2024

View reviewed changes

Merge branch 'main' into fix-train-dreambooth-vae-not-loading

50e5445

improve style

c92f20e

Merge branch 'main' into fix-train-dreambooth-vae-not-loading

34ff831

sayakpaul merged commit cdda94f into huggingface:main May 14, 2024
8 checks passed

bssrdf deleted the fix-train-dreambooth-vae-not-loading branch May 14, 2024 12:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix VAE loading issue in train_dreambooth #7632

fix VAE loading issue in train_dreambooth #7632

bssrdf commented Apr 10, 2024 •

edited

Loading

sayakpaul commented Apr 11, 2024

HuggingFaceDocBuilderDev commented Apr 11, 2024

tolgacangoz commented Apr 11, 2024

bssrdf commented Apr 11, 2024

sayakpaul commented Apr 12, 2024

bssrdf commented Apr 12, 2024

tolgacangoz commented Apr 12, 2024 •

edited

Loading

bssrdf commented Apr 12, 2024

tolgacangoz left a comment

bghira Apr 25, 2024 •

edited

Loading

bssrdf Apr 25, 2024 •

edited

Loading

bghira Apr 25, 2024 •

edited

Loading

bssrdf Apr 28, 2024

bghira Apr 28, 2024

bghira commented Apr 25, 2024 •

edited

Loading

bghira Apr 28, 2024

bssrdf Apr 28, 2024

bghira Apr 28, 2024

bssrdf Apr 28, 2024

sayakpaul left a comment

sayakpaul Apr 29, 2024

bssrdf Apr 29, 2024

sayakpaul Apr 29, 2024

bghira Apr 29, 2024

bssrdf Apr 29, 2024

yiyixuxu commented May 13, 2024

sayakpaul commented May 14, 2024

fix VAE loading issue in train_dreambooth #7632

fix VAE loading issue in train_dreambooth #7632

Conversation

bssrdf commented Apr 10, 2024 • edited Loading

What does this PR do?

Before submitting

Who can review?

sayakpaul commented Apr 11, 2024

HuggingFaceDocBuilderDev commented Apr 11, 2024

tolgacangoz commented Apr 11, 2024

bssrdf commented Apr 11, 2024

sayakpaul commented Apr 12, 2024

bssrdf commented Apr 12, 2024

tolgacangoz commented Apr 12, 2024 • edited Loading

bssrdf commented Apr 12, 2024

tolgacangoz left a comment

Choose a reason for hiding this comment

bghira Apr 25, 2024 • edited Loading

Choose a reason for hiding this comment

bssrdf Apr 25, 2024 • edited Loading

Choose a reason for hiding this comment

bghira Apr 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bghira commented Apr 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sayakpaul left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yiyixuxu commented May 13, 2024

sayakpaul commented May 14, 2024

bssrdf commented Apr 10, 2024 •

edited

Loading

tolgacangoz commented Apr 12, 2024 •

edited

Loading

bghira Apr 25, 2024 •

edited

Loading

bssrdf Apr 25, 2024 •

edited

Loading

bghira Apr 25, 2024 •

edited

Loading

bghira commented Apr 25, 2024 •

edited

Loading