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

Add an option to reduce compile() console spam #23938

Merged
merged 4 commits into from
Jun 2, 2023

Conversation

Rocketknight1
Copy link
Member

This is a very simple PR to add an option to reduce the console spam from our compile() method.

Briefly, Keras model expect you to pass a loss function to compile(). However, transformers models usually compute loss internally. The solution we used was that if the user didn't pass a loss argument to compile(), we would read the model's loss output and use that as the loss. This was non-standard Keras behaviour, though, so we added a warning to make sure users knew what was going on.

Now that we've been doing it for a while, though, that warning is probably just more console spam. This PR adds the option to specify loss="auto", which has exactly the same behaviour but eliminates the warning. The warning also includes a line telling users that they can do that.

@Rocketknight1 Rocketknight1 requested a review from sgugger June 1, 2023 14:14
Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

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

Or maybe we just make the warning an info?

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Jun 1, 2023

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

@Rocketknight1
Copy link
Member Author

I think we tried that, but then most users would miss it! I think we definitely want to make users aware of what's going on (because this is the one big place we diverge from the Keras training standard), but give them an option to disable the warning once they know.

@sgugger
Copy link
Collaborator

sgugger commented Jun 1, 2023

I think proper documentation is a better solution than throwing a warning all the time. Users are pretty mad at us with those already.

@Rocketknight1
Copy link
Member Author

This could be a good opportunity to document stuff, actually! Do you prefer a sidebar tutorial, or some text in the docstring that gets added to all of our TF models?

@sgugger
Copy link
Collaborator

sgugger commented Jun 1, 2023

I think in the official examples where we actually use torch.compile is probably the best place.

@Rocketknight1
Copy link
Member Author

Rocketknight1 commented Jun 1, 2023

This is TF compilation, not torch.compile()! All of our TF examples use it - you can't call model.fit() in Keras before calling model.compile()

@sgugger
Copy link
Collaborator

sgugger commented Jun 1, 2023

Sorry I meant model.compile. And yes, all the examples sounds about right.

@Rocketknight1
Copy link
Member Author

Done! I added a comment when we call compile() in the example scripts. I also fixed up one unnecessary use of run_eagerly in run_mlm.py - oops!

I moved the warning message to logging.info, so it should be invisible to most users now, and they can still pass loss="auto" to disable it entirely if desired.

Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

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

Those and the task examples in the doc too if you don't mind :-)

Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

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

Thanks!

@Rocketknight1 Rocketknight1 force-pushed the less_compile_console_spam branch from 3bf5288 to 34db40c Compare June 2, 2023 14:08
@Rocketknight1 Rocketknight1 merged commit 167a0d8 into main Jun 2, 2023
@Rocketknight1 Rocketknight1 deleted the less_compile_console_spam branch June 2, 2023 14:28
gojiteji pushed a commit to gojiteji/transformers that referenced this pull request Jun 5, 2023
* Add an option to reduce compile() console spam

* Add annotations to the example scripts

* Add notes to the quicktour docs as well

* minor fix
novice03 pushed a commit to novice03/transformers that referenced this pull request Jun 23, 2023
* Add an option to reduce compile() console spam

* Add annotations to the example scripts

* Add notes to the quicktour docs as well

* minor fix
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.

3 participants