Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings
This repository was archived by the owner on Nov 7, 2024. It is now read-only.

Adds pinv to tensorflow backend (https://github.com/google/TensorNetwork/issues/844) #935

Open
krbhanushali wants to merge 3 commits into google:master
base: master
Choose a base branch
Loading
from krbhanushali:master

Conversation

@krbhanushali
Copy link

@krbhanushali krbhanushali commented Aug 31, 2021
edited
Loading

Added pinv to tensorflow_backend.py and corresponding tests to tensorflow_backend_tests,py Issue: #844

Copy link
Contributor

@mganahl mganahl left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, looks good! Could you also add pinv to the AbstractBackend class, as well as numpy and jax backends?

krbhanushali reacted with thumbs up emoji
np.testing.assert_allclose(expected, actual)
expected = tf.linalg.pinv(tensor)
actual = backend.pinv(tensor)
np.testing.assert_allclose(expected, actual) No newline at end of file
Copy link
Contributor

@mganahl mganahl Sep 7, 2021

Choose a reason for hiding this comment

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

add a test that actual @ tensor and tensor @ actual equal the identity (within the desired error)

Copy link
Author

@krbhanushali krbhanushali Sep 8, 2021

Choose a reason for hiding this comment

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

Would testing for all the criteria in https://en.wikipedia.org/wiki/Moore%E2%80%93Penrose_inverse be a better test? (As for an example given in tf's documentation, the product wasn't identity)

"""
return tf.experimental.numpy.finfo(dtype).eps

def pinv(self, tensor: Tensor, rcond: float = 1E-15, hermitian: bool = False) -> Tensor:
Copy link
Contributor

@mganahl mganahl Sep 7, 2021

Choose a reason for hiding this comment

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

I think you can remove the hermitian argument

Copy link
Author

@krbhanushali krbhanushali Sep 8, 2021

Choose a reason for hiding this comment

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

I had included it as it was an argument for numpy. I'll remove it in the next commit.
Thanks for the review!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@mganahl mganahl mganahl requested changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /