Skip to content

Remove save_state_warning in LambdaLR#46813

Closed
jsrozner wants to merge 2 commits into
pytorch:masterfrom
jsrozner:remove_lambdaLR_sched_warning
Closed

Remove save_state_warning in LambdaLR#46813
jsrozner wants to merge 2 commits into
pytorch:masterfrom
jsrozner:remove_lambdaLR_sched_warning

Conversation

@jsrozner

@jsrozner jsrozner commented Oct 24, 2020

Copy link
Copy Markdown
Contributor

Fixes #46405, #43352

I updated the docstring in the local file (function level comments). Do I also need to edit somewhere else or recompile docstrings?

Also, though I didn't change any types here, how is typing (for IDE type checking) documentation generated / used)?

@dr-ci

dr-ci Bot commented Oct 24, 2020

Copy link
Copy Markdown

💊 CI failures summary and remediations

As of commit 7374a75 (more details on the Dr. CI page):


  • 3/3 failures introduced in this PR

🕵️ 3 new failures recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

See CircleCI build pytorch_linux_xenial_cuda9_2_cudnn7_py3_gcc5_4_build (1/3)

Step: "(Optional) Merge target branch" (full log | diagnosis details | 🔁 rerun)

Automatic merge failed; fix conflicts and then commit the result.
CONFLICT (add/add): Merge conflict in .circleci/docker/centos-rocm/Dockerfile 
Auto-merging .circleci/docker/centos-rocm/Dockerfile 
CONFLICT (add/add): Merge conflict in .circleci/config.yml 
Auto-merging .circleci/config.yml 
CONFLICT (add/add): Merge conflict in .circleci/cimodel/data/pytorch_build_definitions.py 
Auto-merging .circleci/cimodel/data/pytorch_build_definitions.py 
CONFLICT (add/add): Merge conflict in .circleci/cimodel/data/pytorch_build_data.py 
Auto-merging .circleci/cimodel/data/pytorch_build_data.py 
CONFLICT (add/add): Merge conflict in .circleci/cimodel/data/dimensions.py 
Auto-merging .circleci/cimodel/data/dimensions.py 
Automatic merge failed; fix conflicts and then commit the result. 

See CircleCI build pytorch_xla_linux_bionic_py3_6_clang9_build (2/3)

Step: "(Optional) Merge target branch" (full log | diagnosis details | 🔁 rerun)

Automatic merge failed; fix conflicts and then commit the result.
CONFLICT (add/add): Merge conflict in .circleci/docker/centos-rocm/Dockerfile 
Auto-merging .circleci/docker/centos-rocm/Dockerfile 
CONFLICT (add/add): Merge conflict in .circleci/config.yml 
Auto-merging .circleci/config.yml 
CONFLICT (add/add): Merge conflict in .circleci/cimodel/data/pytorch_build_definitions.py 
Auto-merging .circleci/cimodel/data/pytorch_build_definitions.py 
CONFLICT (add/add): Merge conflict in .circleci/cimodel/data/pytorch_build_data.py 
Auto-merging .circleci/cimodel/data/pytorch_build_data.py 
CONFLICT (add/add): Merge conflict in .circleci/cimodel/data/dimensions.py 
Auto-merging .circleci/cimodel/data/dimensions.py 
Automatic merge failed; fix conflicts and then commit the result. 

See CircleCI build pytorch_linux_xenial_py3_6_gcc5_4_build (3/3)

Step: "(Optional) Merge target branch" (full log | diagnosis details | 🔁 rerun)

Automatic merge failed; fix conflicts and then commit the result.
CONFLICT (add/add): Merge conflict in .circleci/docker/centos-rocm/Dockerfile 
Auto-merging .circleci/docker/centos-rocm/Dockerfile 
CONFLICT (add/add): Merge conflict in .circleci/config.yml 
Auto-merging .circleci/config.yml 
CONFLICT (add/add): Merge conflict in .circleci/cimodel/data/pytorch_build_definitions.py 
Auto-merging .circleci/cimodel/data/pytorch_build_definitions.py 
CONFLICT (add/add): Merge conflict in .circleci/cimodel/data/pytorch_build_data.py 
Auto-merging .circleci/cimodel/data/pytorch_build_data.py 
CONFLICT (add/add): Merge conflict in .circleci/cimodel/data/dimensions.py 
Auto-merging .circleci/cimodel/data/dimensions.py 
Automatic merge failed; fix conflicts and then commit the result. 

This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 4 times.

@izdeby
izdeby requested a review from vincentqb October 26, 2020 17:13
@izdeby izdeby added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Oct 26, 2020
@facebook-github-bot

Copy link
Copy Markdown
Contributor

Hi @jsrozner!

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@vincentqb

Copy link
Copy Markdown
Contributor

I updated the docstring in the local file (function level comments). Do I also need to edit somewhere else or recompile docstrings?

As mentioned in comment, the docstring will automatically regenerate the doc on master. :)

Also, though I didn't change any types here, how is typing (for IDE type checking) documentation generated / used)?

Since this is just updating the warning and the documentation, there is no need update the typing.

@jsrozner

Copy link
Copy Markdown
Contributor Author

Also, though I didn't change any types here, how is typing (for IDE type checking) documentation generated / used)?

Since this is just updating the warning and the documentation, there is no need update the typing.

Yes, agreed. But I was curious how the pytorch library handles typing since it is not inlined in, e.g., these function definitions

Is anything else needed by me to finish this pull request?

@vincentqb

Copy link
Copy Markdown
Contributor

Also, though I didn't change any types here, how is typing (for IDE type checking) documentation generated / used)?

Since this is just updating the warning and the documentation, there is no need update the typing.

Yes, agreed. But I was curious how the pytorch library handles typing since it is not inlined in, e.g., these function definitions

We are migrating to inline, but the types are in *.pyi files adjacent :)

Is anything else needed by me to finish this pull request?

Thanks for signing and writing the pull request, this is good to go :)

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

LGTM :)

@facebook-github-bot facebook-github-bot 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.

@vincentqb has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@jsrozner

jsrozner commented Dec 1, 2020

Copy link
Copy Markdown
Contributor Author

@vincentqb
Can we merge?

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@vincentqb merged this pull request in 42e6951.

@stas00

stas00 commented Dec 8, 2020

Copy link
Copy Markdown
Contributor

That broke transformers where it's used:
https://github.com/huggingface/transformers/blob/aa0969c2f3cf5cf7ce5690e7b77f95b1622988d6/src/transformers/trainer_pt_utils.py#L37

But reading why it was done (goodness), transformers will just need to recode for this change.

Not sure how one could deprecate a constant...

stas00 added a commit to huggingface/transformers that referenced this pull request Dec 8, 2020
FYI `SAVE_STATE_WARNING` has been removed 3 days ago: pytorch/pytorch#46813

Fixes: #8232

@sgugger
stas00 added a commit to huggingface/transformers that referenced this pull request Dec 8, 2020
* [training] SAVE_STATE_WARNING was removed in pytorch

FYI `SAVE_STATE_WARNING` has been removed 3 days ago: pytorch/pytorch#46813

Fixes: #8232

@sgugger

* style, but add () to prevent autoformatters from botching it

* switch to try/except

* cleanup
binshengliu pushed a commit to binshengliu/transformers that referenced this pull request Mar 5, 2021
* [training] SAVE_STATE_WARNING was removed in pytorch

FYI `SAVE_STATE_WARNING` has been removed 3 days ago: pytorch/pytorch#46813

Fixes: huggingface#8232

@sgugger

* style, but add () to prevent autoformatters from botching it

* switch to try/except

* cleanup
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
Summary:
Fixes pytorch#46405, pytorch#43352

I updated the docstring in the local file (function level comments). Do I also need to edit somewhere else or recompile docstrings?

Also, though I didn't change any types here, how is typing (for IDE type checking) documentation generated / used)?

Pull Request resolved: pytorch#46813

Reviewed By: ezyang

Differential Revision: D24923112

Pulled By: vincentqb

fbshipit-source-id: be7818e0d4593bfc5d74023b9c361ac2a538589a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla signed Merged open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow downstream users to suppress Save Optimizer warnings

6 participants