Skip to content

Add determinant function on variable; Add backward on svd #3816

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
Merged
soumith merged 2 commits into from
Dec 1, 2017
Merged

Add determinant function on variable; Add backward on svd #3816

soumith merged 2 commits into from
Dec 1, 2017

Conversation

@ssnl
Copy link
Collaborator

@ssnl ssnl commented Nov 21, 2017

Addresses #3423.

On why not to bp through svd: there is no concrete formula when singular values are not distinct.

@pytorchbot
Copy link
Collaborator

pytorchbot commented Nov 21, 2017

@ssnl, thanks for your PR! We identified @zdevito to be a potential reviewer.

@pietern
Copy link
Contributor

pietern commented Nov 21, 2017

Snafu on Jenkins workers... The first build failure is not a real failure.

@pytorchbot retest this please

@ssnl
Copy link
Collaborator Author

ssnl commented Nov 21, 2017

Hmm tests are failing for Lapack not found even if I set skipIfNoLapack. Looking into this.

zdevito
zdevito reviewed Nov 21, 2017
View reviewed changes
torch/autograd/variable.py Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

aten/src/ATen/native/native_functions.yaml Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

@ssnl
Copy link
Collaborator Author

ssnl commented Nov 22, 2017

The errors are from the matrix generation function which uses lapack. I will fix those.

@ssnl
Copy link
Collaborator Author

ssnl commented Nov 22, 2017

Fixed the tests.

@ssnl ssnl changed the title Add determinant function on variable Add determinant function on variable; Add backward on svd Nov 30, 2017
@ssnl
Copy link
Collaborator Author

ssnl commented Nov 30, 2017

Added backward support for svd, hence enabling double backward on det. Cases where this can be unstable are described in doc.

@ssnl ssnl force-pushed the det branch 2 times, most recently from 59e716d to fb043e3 Compare November 30, 2017 00:16
@ssnl
Copy link
Collaborator Author

ssnl commented Nov 30, 2017

The code is not compatible with changes to gen_variable_type. Will fix tomorrow.

@ssnl ssnl force-pushed the det branch 2 times, most recently from 2f66f03 to 7b5fcce Compare November 30, 2017 16:44
colesbury
colesbury approved these changes Dec 1, 2017
View reviewed changes
Copy link
Member

@colesbury colesbury left a comment

Choose a reason for hiding this comment

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

lgtm

@soumith soumith merged commit c681b03 into pytorch:master Dec 1, 2017
@soumith soumith mentioned this pull request Dec 1, 2017
Closed
@ssnl ssnl deleted the det branch December 1, 2017 18:23
@CarloLucibello CarloLucibello mentioned this pull request Feb 17, 2018
Open
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zdevito zdevito zdevito left review comments

@colesbury colesbury colesbury approved these changes

Assignees

No one assigned

Labels

open source

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@ssnl @pytorchbot @pietern @zdevito @colesbury @ezyang @soumith