Skip to content

[textual inversion] Allow validation images - #2077

Merged
pcuenca merged 4 commits into
mainfrom
textual-inversion-eval
Jan 26, 2023
Merged

pcuenca merged 4 commits into
mainfrom
textual-inversion-eval

Conversation

@pcuenca

@pcuenca pcuenca commented Jan 23, 2023

Copy link
Copy Markdown
Member

No description provided.

@HuggingFaceDocBuilderDev

HuggingFaceDocBuilderDev commented Jan 23, 2023 •

Copy link
Copy Markdown

The documentation is not available anymore as the PR was closed or merged.

@patrickvonplaten patrickvonplaten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks a lot!

@pcuenca

pcuenca commented Jan 24, 2023

Copy link
Copy Markdown
Member Author

Oops, I wrote this yesterday while I was working on a different textual inversion fix, but did not realize #1838 existed. @patil-suraj we can close this one and keep working on #1838 as it was first, or we can add @isamu-isozaki as an author here, what do you think?

@isamu-isozaki

Copy link
Copy Markdown
Contributor

@pcuenca Personally happy with either! I think there's a slight bug when using mixed_precision=fp16 because the text encoder will be in fp32 and everything else is in fp16. Also, probably assigning unet=unet, vae=vae in the pipeline is better because it uses less memory. For my gpu, I did keep getting oom when I didn't do that.

@isamu-isozaki

isamu-isozaki commented Jan 24, 2023 •

Copy link
Copy Markdown
Contributor

Also, very minor but logging/validation with global_step might be cleaner since we are doing that for save anyways

@patil-suraj patil-suraj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me! Think we can add isamu-isozaki as co-author and merge this PR.

@pcuenca

pcuenca commented Jan 26, 2023

Copy link
Copy Markdown
Member Author

@pcuenca Personally happy with either! I think there's a slight bug when using mixed_precision=fp16 because the text encoder will be in fp32 and everything else is in fp16.

I think that's only true if we reuse the unet and vae like you suggest. Otherwise, from_pretrained will load everything in float32, as this comment explains: https://github.com/huggingface/diffusers/pull/2077/files#diff-a3d54a67360d6c7648a2de57fea2d72444395d0ba56eb4ba8545c6ca820c3f47R776

Also, probably assigning unet=unet, vae=vae in the pipeline is better because it uses less memory. For my gpu, I did keep getting oom when I didn't do that.

Yeah, in that case we'd need to load the pipeline setting torch_dtype to the half precision type.

I'm hesitant about doing this here because all the other training scripts use the same approach, and in general diffusers defaults to running inference using the highest precision. Maybe open a new PR to discuss the implications, or make it an optional argument for validation?

@pcuenca

pcuenca commented Jan 26, 2023

Copy link
Copy Markdown
Member Author

Also, very minor but logging/validation with global_step might be cleaner since we are doing that for save anyways

I do agree with this, it's weird to me that the frequency of some tasks is determined in epochs while others are expressed in steps. Would you be willing to open an issue/PR to make it consistent in all training scripts?

@pcuenca

pcuenca commented Jan 26, 2023

Copy link
Copy Markdown
Member Author

Merging as is, but those are some valuable comments, @isamu-isozaki! Let's try to deal with them in new PRs so we can discuss and apply any changes to all training scripts :) Thanks a lot! 馃檶

@pcuenca
pcuenca merged commit 0856137 into main Jan 26, 2023
@pcuenca
pcuenca deleted the textual-inversion-eval branch January 26, 2023 08:20
@isamu-isozaki

Copy link
Copy Markdown
Contributor

@pcuenca Thanks for the comments! Will make new issues for both to discuss

AmericanPresidentJimmyCarter pushed a commit to AmericanPresidentJimmyCarter/diffusers that referenced this pull request Apr 26, 2024
* [textual inversion] Allow validation images.

* Change key to `validation`

* Specify format instead of transposing.

As discussed with @sayakpaul.

* Style

Co-authored-by: isamu-isozaki <isamu.website@gmail.com>
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.

5 participants