Skip to content

Port SVD to ATen, enable batching for matrix inputs #21588

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 19 commits into from
Closed

Port SVD to ATen, enable batching for matrix inputs #21588

vishwakftw wants to merge 19 commits into from

Conversation

@vishwakftw
Copy link
Contributor

@vishwakftw vishwakftw commented Jun 10, 2019 •
edited
Loading

Changelog:

  • Port SVD TH implementation to ATen/native/BatchLinearAlgebra.cpp
  • Port SVD THC implementation to ATen/native/cuda/BatchLinearAlgebra.cu
  • Allow batches of matrices as arguments to torch.svd
  • Remove existing implementations in TH and THC
  • Update doc string
  • Update derivatives to support batching
  • Modify nuclear norm implementation to use at::svd instead of _batch_svd
  • Remove _batch_svd as it is redundant

Test Plan:

  • Add new test suite for SVD in test_torch.py with port to test_cuda.py
  • Add tests in common_methods_invocations.py for derivative testing

@pytorchbot pytorchbot added module: cpu CPU specific problem (e.g., perf, algorithm) module: internals Related to internal abstractions in c10 and ATen module: operators labels Jun 10, 2019
@gpleiss gpleiss mentioned this pull request Jun 10, 2019
Closed
14 tasks
@vishwakftw
Copy link
Contributor Author

vishwakftw commented Jun 10, 2019

Test failures are expected. I removed the TH bindings.

@vishwakftw vishwakftw closed this Jun 11, 2019
@vishwakftw vishwakftw reopened this Jun 21, 2019
@pytorchbot pytorchbot added module: cublas Problem related to cublas support module: cuda Related to torch.cuda, and CUDA support in general labels Jun 21, 2019
@vishwakftw vishwakftw closed this Jun 21, 2019
@vishwakftw vishwakftw reopened this Jun 21, 2019
@vishwakftw
Copy link
Contributor Author

vishwakftw commented Jun 22, 2019

I’ll investigate the test failures, they are related to this PR.

@pytorchbot pytorchbot added module: docs Related to our documentation, both in docs/ and docblocks module: tests Issues related to tests (not the torch.testing module) labels Jul 1, 2019
@vishwakftw vishwakftw changed the title [WIP] Port SVD to ATen, enable batching for matrix inputs Port SVD to ATen, enable batching for matrix inputs Jul 1, 2019
@zhangguanheng66 zhangguanheng66 requested a review from gchanan July 1, 2019 22:14
@zhangguanheng66 zhangguanheng66 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jul 1, 2019
@vishwakftw
Copy link
Contributor Author

vishwakftw commented Jul 7, 2019

@pytorchbot rebase this please

@vishwakftw
Copy link
Contributor Author

vishwakftw commented Jul 10, 2019

@pytorchbot rebase this please

@pytorchbot pytorchbot added the module: pybind Related to our Python bindings / interactions with other Python libraries label Jul 11, 2019
@gchanan gchanan requested a review from nairbv July 11, 2019 18:47
@gchanan
Copy link
Contributor

gchanan commented Jul 11, 2019

@nairbv can you review this, please?

nairbv
nairbv reviewed Jul 12, 2019
View reviewed changes
nairbv
nairbv reviewed Jul 12, 2019
View reviewed changes
nairbv
nairbv reviewed Jul 12, 2019
View reviewed changes
@vishwakftw
Code Review - I
34bf0bb 
- Add a comment about why empty tensors are created on the CPU while the input is a CUDA tensor in _create_U_S_VT
- Print matrix in SVD doc example
- strides() --> stride() in docs
@vishwakftw
Merge branch 'master' of https://github.com/pytorch/pytorch into batc…
abe16ac 
…h-svd
@nairbv
Copy link
Collaborator

nairbv commented Jul 15, 2019

@pytorchbot rebase this please

@nairbv
Copy link
Collaborator

nairbv commented Jul 15, 2019

looks like failing tests were due to whatever this fixed:
b5fa9a3

@vishwakftw
Copy link
Contributor Author

vishwakftw commented Jul 15, 2019

Yes, I’ll manually rebase.

@nairbv
Copy link
Collaborator

nairbv commented Jul 15, 2019

LGTM

facebook-github-bot
facebook-github-bot reviewed Jul 15, 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.

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

zdevito pushed a commit to zdevito/ATen that referenced this pull request Jul 15, 2019
@vishwakftw @facebook-github-bot
Port SVD to ATen, enable batching for matrix inputs (#21588)
ea8e3dd 
Summary:
Changelog:
- Port SVD TH implementation to ATen/native/BatchLinearAlgebra.cpp
- Port SVD THC implementation to ATen/native/cuda/BatchLinearAlgebra.cu
- Allow batches of matrices as arguments to `torch.svd`
- Remove existing implementations in TH and THC
- Update doc string
- Update derivatives to support batching
- Modify nuclear norm implementation to use at::svd instead of _batch_svd
- Remove _batch_svd as it is redundant
Pull Request resolved: pytorch/pytorch#21588

Test Plan:
- Add new test suite for SVD in test_torch.py with port to test_cuda.py
- Add tests in common_methods_invocations.py for derivative testing

Differential Revision: D16266115

Pulled By: nairbv

fbshipit-source-id: e89bb0dbd8f2d58bd758b7830d2389c477aa61fb
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jul 15, 2019

@nairbv merged this pull request in 7d055c2.

@vishwakftw vishwakftw deleted the batch-svd branch July 23, 2019 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

@nairbv nairbv nairbv approved these changes

@gchanan gchanan Awaiting requested review from gchanan

Assignees

No one assigned

Labels

Merged module: cpu CPU specific problem (e.g., perf, algorithm) module: cublas Problem related to cublas support module: cuda Related to torch.cuda, and CUDA support in general module: docs Related to our documentation, both in docs/ and docblocks module: internals Related to internal abstractions in c10 and ATen module: pybind Related to our Python bindings / interactions with other Python libraries 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.

8 participants

@vishwakftw @gchanan @nairbv @facebook-github-bot @ezyang @zhangguanheng66 @pytorchbot @mruberry