Skip to content

Catch PyTorch warning when saving/loading scheduler#7401

Merged
sgugger merged 1 commit into
masterfrom
catch_pt_warning
Sep 28, 2020
Merged

Catch PyTorch warning when saving/loading scheduler#7401
sgugger merged 1 commit into
masterfrom
catch_pt_warning

Conversation

@sgugger

@sgugger sgugger commented Sep 25, 2020

Copy link
Copy Markdown
Collaborator

When saving or loading the scheduler, PyTorch always sends a warning to save/load the optimizer state as well (with a typo). We are saving/loading the optimizer state along the scheduler but there is no way to tell that to PyTorch and avoid the annoying warning (and its typo).

This PR fixes that by catching all warnings while loading/saving the scheduler then reissuing the non-expected ones.

@codecov

codecov Bot commented Sep 25, 2020

Copy link
Copy Markdown

Codecov Report

Merging #7401 into master will increase coverage by 2.28%.
The diff coverage is 13.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7401      +/-   ##
==========================================
+ Coverage   77.06%   79.35%   +2.28%     
==========================================
  Files         181      181              
  Lines       35781    35793      +12     
==========================================
+ Hits        27575    28403     +828     
+ Misses       8206     7390     -816
Impacted Files Coverage Δ
src/transformers/trainer.py 55.01% <13.33%> (-0.70%) ⬇️
src/transformers/modeling_tf_longformer.py 17.46% <0.00%> (-81.13%) ⬇️
src/transformers/modeling_layoutlm.py 25.06% <0.00%> (-69.40%) ⬇️
src/transformers/modeling_tf_xlm.py 58.52% <0.00%> (-34.74%) ⬇️
src/transformers/configuration_layoutlm.py 80.00% <0.00%> (-20.00%) ⬇️
src/transformers/modeling_roberta.py 81.51% <0.00%> (-15.11%) ⬇️
src/transformers/modeling_mobilebert.py 79.31% <0.00%> (-10.12%) ⬇️
src/transformers/generation_tf_utils.py 86.71% <0.00%> (+0.25%) ⬆️
src/transformers/modeling_tf_utils.py 87.22% <0.00%> (+0.31%) ⬆️
src/transformers/modeling_bart.py 94.61% <0.00%> (+0.33%) ⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e50a931...864dd99. Read the comment docs.

@julien-c

julien-c commented Sep 25, 2020

Copy link
Copy Markdown
Member

worth pushing upstream to pytorch?

@sgugger

sgugger commented Sep 27, 2020

Copy link
Copy Markdown
Collaborator Author

We can certainly file an issue about it but my guess is that they though a warning always passed was fine (since there is no way to know if the user is saving/loading the optimizer with its scheduler).

LysandreJik
LysandreJik approved these changes Sep 28, 2020
View reviewed changes

@LysandreJik LysandreJik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice workaround!

@sgugger
sgugger merged commit 7563d5a into master Sep 28, 2020
@sgugger
sgugger deleted the catch_pt_warning branch September 28, 2020 12:20
@jsrozner

jsrozner commented Oct 15, 2020

Copy link
Copy Markdown
Contributor

Thanks for this!

Regarding whether to push upstream to pytorch:
maybe a solution is to add optional flag to the pytorch save command like optimizer_was_saved. Make it default False. Only if you explicitly mark the param true in your call to save the optimizer will the warning be suppressed. Puts all the onus on the calling user.

This was referenced Oct 15, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LysandreJik LysandreJik LysandreJik approved these changes
@julien-c julien-c Awaiting requested review from julien-c

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sgugger @julien-c @jsrozner @LysandreJik