Skip to content

Conversation

@kn
Copy link
Contributor

@kn kn commented Dec 19, 2022

Problem:
xformers being available doesn't always mean it should be used. For example, I install xformers because it makes inference fast but I find xformers make my dreambooth training slower.

Solution:
Require explicit flag to enable xformers when running training scripts.

Related recent change: #1640

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Dec 19, 2022

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

@patrickvonplaten
Copy link
Contributor

cc @patil-suraj @pcuenca, I tend to agree here with @kn . If we make it fully optional we should however not wrap it into a "try .... else ..." statement, but just let it fail if it's not available

Copy link
Member

@pcuenca pcuenca left a comment

Choose a reason for hiding this comment

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

Thanks, I think this makes sense!

@kn
Copy link
Contributor Author

kn commented Dec 19, 2022

good idea on raising an exception when xformers is used but not available! PTAL

@patrickvonplaten
Copy link
Contributor

I'm in favor of this as it better follows the "no optimizations by default" API of PyTorch and should be "less surprising" for the user long-term.

This change might suddenly slow down some notebooks, so also kindly pinging @apolinario and @patil-suraj here.

Copy link
Contributor

@patil-suraj patil-suraj left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the PR, agree with it, makes sense to add an explicit argument here to enable exformers. Just left a nit. Also, let's add a note in the examples readme about this new argument.

),
)
parser.add_argument("--local_rank", type=int, default=-1, help="For distributed training: local_rank")
parser.add_argument("--use_xformers", action="store_true", help="Whether or not to use xformers.")
Copy link
Contributor

@patil-suraj patil-suraj Dec 23, 2022

Choose a reason for hiding this comment

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

(nit) could we call this arg enable_xformers_memory_efficient_attention to align with the method name?

@kn kn force-pushed the make-xformers-optional branch from 465c1d6 to 8dc93ae Compare December 27, 2022 18:37
@kn
Copy link
Contributor Author

kn commented Dec 27, 2022

renamed the flag and added a note in READMEs - PTAL

Copy link
Contributor

@patil-suraj patil-suraj left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@patil-suraj patil-suraj merged commit 8874027 into huggingface:main Dec 27, 2022
@patrickvonplaten
Copy link
Contributor

Thanks for the PR @kn !

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