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

Parity tests for functional optimizer step_param #61756

Closed
wants to merge 16 commits into from

Conversation

rohan-varma
Copy link
Member

@rohan-varma rohan-varma commented Jul 16, 2021

Stack from ghstack:

DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function step_param.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.

Differential Revision: D29727549

NOTE FOR REVIEWERS: This PR has internal Facebook specific changes or comments, please review them on Phabricator!

DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jul 16, 2021

🔗 Helpful links

💊 CI failures summary and remediations

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


  • 1/1 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

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

See GitHub Actions build Linux CI (pytorch-linux-bionic-py3.8-gcc9-coverage) / render_test_results (default) (1/1)

Step: "Checkout PyTorch" (full log | diagnosis details | 🔁 rerun)

rm: cannot remove '/home/ec2-user/actions-runne..._pipelines/verify-pipeline.yml': Permission denied
rm: cannot remove '/home/ec2-user/actions-runner/_work/pytorch/pytorch/.azure_pipelines/job_templates/prepare-build-template.yml': Permission denied
rm: cannot remove '/home/ec2-user/actions-runner/_work/pytorch/pytorch/.azure_pipelines/job_templates/pytorch-template-unix.yml': Permission denied
rm: cannot remove '/home/ec2-user/actions-runner/_work/pytorch/pytorch/.azure_pipelines/job_templates/pytorch-template-win.yml': Permission denied
rm: cannot remove '/home/ec2-user/actions-runner/_work/pytorch/pytorch/.azure_pipelines/job_templates/set-environment-variables.yml': Permission denied
rm: cannot remove '/home/ec2-user/actions-runner/_work/pytorch/pytorch/.azure_pipelines/job_templates/wheel-wait-job-template.yml': Permission denied
rm: cannot remove '/home/ec2-user/actions-runner/_work/pytorch/pytorch/.azure_pipelines/job_templates/wheel-wait-template.yml': Permission denied
rm: cannot remove '/home/ec2-user/actions-runner/_work/pytorch/pytorch/.azure_pipelines/build-pipeline.yml': Permission denied
rm: cannot remove '/home/ec2-user/actions-runner/_work/pytorch/pytorch/.azure_pipelines/daily-pipeline.yml': Permission denied
rm: cannot remove '/home/ec2-user/actions-runner/_work/pytorch/pytorch/.azure_pipelines/nightly-pytorch-tests-pipeline.yml': Permission denied
rm: cannot remove '/home/ec2-user/actions-runner/_work/pytorch/pytorch/.azure_pipelines/pytorch-tests-pipeline.yml': Permission denied
rm: cannot remove '/home/ec2-user/actions-runner/_work/pytorch/pytorch/.azure_pipelines/verify-pipeline.yml': Permission denied

2021-07-26T21:45:45.7593602Z Post job cleanup.
2021-07-26T21:45:45.8384744Z Cleaning up orphan processes


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 to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!

[ghstack-poisoned]
rohan-varma added a commit that referenced this pull request Jul 16, 2021
Pull Request resolved: #61756

DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.
ghstack-source-id: 133659711

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!
DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!

[ghstack-poisoned]
rohan-varma added a commit that referenced this pull request Jul 16, 2021
Pull Request resolved: #61756

DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.
ghstack-source-id: 133682132

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!
DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!

[ghstack-poisoned]
DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!

[ghstack-poisoned]
rohan-varma added a commit that referenced this pull request Jul 20, 2021
Pull Request resolved: #61756

DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.
ghstack-source-id: 133919961

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!
# Functional optimizer step_param
for param in module_functional.parameters():
grad = param.grad
optim_functional.step_param(param, grad)
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that the first arg of step_param should be a tensor instead of nn.parameter.Parameter. Is there an implicit type casting here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point! According to these docs: https://github.com/pytorch/pytorch/blob/master/torch/nn/parameter.py#L7 a Parameter is a kind of tensor (and also subclasses from Tensor), so I think step_param should work regardless of whether it is passed a parameter or tensor.

DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!

[ghstack-poisoned]
rohan-varma added a commit that referenced this pull request Jul 20, 2021
Pull Request resolved: #61756

DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.
ghstack-source-id: 133945706

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!
DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!

[ghstack-poisoned]
DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!

[ghstack-poisoned]
rohan-varma added a commit that referenced this pull request Jul 22, 2021
Pull Request resolved: #61756

DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.
ghstack-source-id: 134052041

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!
DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!

[ghstack-poisoned]
DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!

[ghstack-poisoned]
DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!

[ghstack-poisoned]
DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!

[ghstack-poisoned]
DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!

[ghstack-poisoned]
DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!

[ghstack-poisoned]
rohan-varma added a commit that referenced this pull request Jul 25, 2021
Pull Request resolved: #61756

DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.
ghstack-source-id: 134272866

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!
DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!

[ghstack-poisoned]
DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 69adb21.

@facebook-github-bot facebook-github-bot deleted the gh/rohan-varma/355/head branch July 30, 2021 14:27
jasperzhong pushed a commit to jasperzhong/swift that referenced this pull request Nov 25, 2021
Pull Request resolved: pytorch/pytorch#61756

DDP will support running optimizer as communication hook with
optimizers that support a per-parameter/gradient step function `step_param`.
Add parity tests as we implement more optimizers that support step_param to
ensure parity with regular optimizers.
ghstack-source-id: 133985151

Differential Revision: [D29727549](https://our.internmc.facebook.com/intern/diff/D29727549/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D29727549/)!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants