Skip to content

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

Closed
vishwakftw wants to merge 19 commits intopytorch:masterfrom
vishwakftw:batch-svd
Closed

Port SVD to ATen, enable batching for matrix inputs#21588
vishwakftw wants to merge 19 commits intopytorch:masterfrom
vishwakftw:batch-svd

Conversation

@vishwakftw
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

vishwakftw commented Jul 7, 2019

@pytorchbot rebase this please

@vishwakftw
Copy link
Copy Markdown
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
Copy Markdown
Contributor

gchanan commented Jul 11, 2019

@nairbv can you review this, please?

nairbv
nairbv reviewed Jul 12, 2019
View reviewed changes
Comment thread aten/src/ATen/native/LinearAlgebraUtils.h
nairbv
nairbv reviewed Jul 12, 2019
View reviewed changes
Comment thread torch/_torch_docs.py
nairbv
nairbv reviewed Jul 12, 2019
View reviewed changes
Comment thread torch/_torch_docs.py Outdated
@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
Copy Markdown
Collaborator

nairbv commented Jul 15, 2019

@pytorchbot rebase this please

@nairbv
Copy link
Copy Markdown
Collaborator

nairbv commented Jul 15, 2019

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

@vishwakftw
Copy link
Copy Markdown
Contributor Author

vishwakftw commented Jul 15, 2019

Yes, I’ll manually rebase.

@nairbv
Copy link
Copy Markdown
Collaborator

nairbv commented Jul 15, 2019

LGTM

facebook-github-bot
facebook-github-bot reviewed Jul 15, 2019
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
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
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
@vishwakftw @facebook-github-bot
Port SVD to ATen, enable batching for matrix inputs (pytorch#21588)
00a41f5 
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#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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nairbv nairbv nairbv approved these changes

@gchanan gchanan Awaiting requested review from gchanan

+1 more reviewer

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

Reviewers whose approvals may not affect merge requirements

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