Skip to content

Add derivative for QR decomposition #21274

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.

Sign up for GitHub

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

Jump to bottom
Closed
vishwakftw wants to merge 2 commits into from
Closed

Add derivative for QR decomposition #21274

vishwakftw wants to merge 2 commits into from

Conversation

@vishwakftw
Copy link
Contributor

@vishwakftw vishwakftw commented Jun 3, 2019

Changelog:

  • Implement derivative for QR decomposition for tall and square matrices i.e., num rows >= num cols

Test Plan:

  • Add tests for QR decomposition derivative in common_methods_invocations.py

@vishwakftw
Add derivative for QR decomposition
8b4d824 
Changelog:
- Implement derivative for QR decomposition for tall and square matrices i.e., num rows >= num cols

Test Plan:
- Add tests for QR decomposition derivative in common_methods_invocations.py
@pytorchbot pytorchbot added module: internals Related to internal abstractions in c10 and ATen module: tests Issues related to tests (not the torch.testing module) labels Jun 3, 2019
@vishwakftw
Copy link
Contributor Author

vishwakftw commented Jun 3, 2019

I believe pending builds / tests have passed with this message:
GitHub rate limited us while adding a commit status of success

@jerryzh168 jerryzh168 requested review from ailzhang, gchanan and zdevito and removed request for zdevito June 4, 2019 18:40
@jerryzh168 jerryzh168 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jun 4, 2019
ailzhang
ailzhang reviewed Jun 4, 2019
View reviewed changes
Copy link
Contributor

@ailzhang ailzhang left a comment

Choose a reason for hiding this comment

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

Didn't check the backward func carefully, but the way tests were added make sense(it doesn't break autodiff tests).

@vishwakftw
Copy link
Contributor Author

vishwakftw commented Jun 5, 2019

@pytorchbot rebase this please

facebook-github-bot
facebook-github-bot reviewed Jun 6, 2019
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@ezyang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jun 6, 2019

@ezyang merged this pull request in 6615797.

@vishwakftw vishwakftw deleted the grad-for-qr branch June 6, 2019 17:50
@ezyang ezyang mentioned this pull request Jun 24, 2019
Closed
52 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ailzhang ailzhang ailzhang left review comments

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

@ezyang ezyang ezyang approved these changes

@gchanan gchanan Awaiting requested review from gchanan

Assignees

No one assigned

Labels

Merged module: internals Related to internal abstractions in c10 and ATen module: tests Issues related to tests (not the torch.testing module) open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@vishwakftw @facebook-github-bot @ezyang @ailzhang @jerryzh168 @pytorchbot @mruberry