Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[SD3 LoRA Training] Fix errors when not training text encoders #8743

Merged
merged 6 commits into from
Jul 2, 2024

Conversation

asomoza
Copy link
Member

@asomoza asomoza commented Jun 30, 2024

What does this PR do?

Adds some validations to the SD3 dreambooth lora training when not training the text encoders.

Fixes the following errors:

train command:

accelerate launch examples/dreambooth/train_dreambooth_lora_sd3.py --pretrained_model_name_or_path "hf-internal-testing/tiny-sd3-pipe" --instance_data_dir "docs/source/en/imgs" --instance_prompt "photo" --resolution 64 --train_batch_size 1 --gradient_accumulation_steps 1 --max_train_steps 2 --learning_rate 5.0e-04 --scale_lr --lr_scheduler constant --lr_warmup_steps 0 --output_dir "outputs/temp"
text_input_ids=text_input_ids_list[i],
                   ~~~~~~~~~~~~~~~~~~~^^^
TypeError: 'NoneType' object is not subscriptable

and

text_lora_parameters_one,
    ^^^^^^^^^^^^^^^^^^^^^^^^
UnboundLocalError: cannot access local variable 'text_lora_parameters_one' where it is not associated with a value

Note: I only added validations when the variables are None, didn't check the logic or the result of the training.

Who can review?

@sayakpaul @linoytsaban

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

@asomoza asomoza requested review from linoytsaban and sayakpaul June 30, 2024 11:11
Copy link
Member

@sayakpaul sayakpaul left a comment

Choose a reason for hiding this comment

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

Thanks for the changes. Left some comments.

@sayakpaul
Copy link
Member

@asomoza let's apply the changes from the review comments then. Before merging, let's also perhaps do two runs -- with and without text encoder training.

@linoytsaban
Copy link
Collaborator

linoytsaban commented Jul 1, 2024

@sayakpaul @asomoza btw just tried an experiment only with the change of text_input_ids=text_input_ids_list[-1] which didn't resolve the issue with text encoder training using instance prompt(#8726) (when using custom prompts it works fine) - just fyi in case it helps us investigate the error

sayakpaul and others added 2 commits July 1, 2024 18:02
@sayakpaul
Copy link
Member

@asomoza

@linoytsaban and I took the liberty of fixing some things. Hope that is fine.

I have run some basic training on my end and things seemed to be working. It'd be nice if you could verify that as well (Linoy, Alvaro, both).

@neuron-party could you also check if this PR resolves the problems you were facing?

@asomoza
Copy link
Member Author

asomoza commented Jul 1, 2024

@linoytsaban @sayakpaul I tested it with just instance_prompt and train_text_encoder and it worked for me using this PR. I can't do a full training right now but I'll test it later.

@sayakpaul
Copy link
Member

Thanks! We can merge afterward then. Let us know.

@asomoza
Copy link
Member Author

asomoza commented Jul 1, 2024

Tested a full train with the dog and IMO it's better than before:

Without TE 20240701123018_821636139 20240701123028_3315018697
With TE 20240701134128_2403635654 20240701134148_1745400716

So I think we're good to merge

@linoytsaban
Copy link
Collaborator

Thanks @asomoza! looks good 🔥

@sayakpaul sayakpaul merged commit af92869 into huggingface:main Jul 2, 2024
8 checks passed
@sayakpaul
Copy link
Member

Thanks all :)

@asomoza asomoza deleted the fix-sd3-db-lora-training branch July 2, 2024 02:42
sayakpaul added a commit that referenced this pull request Dec 23, 2024
* fix

* fix things.

Co-authored-by: Linoy Tsaban <[email protected]>

* remove patch

* apply suggestions

---------

Co-authored-by: Linoy Tsaban <[email protected]>
Co-authored-by: sayakpaul <[email protected]>
Co-authored-by: Linoy Tsaban <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants