Skip to content
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.

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

Add Cholesky decomposition #852

Open
alewis opened this issue Oct 12, 2020 · 17 comments · May be fixed by #937
Open

Add Cholesky decomposition #852

alewis opened this issue Oct 12, 2020 · 17 comments · May be fixed by #937
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@alewis
Copy link
Collaborator

alewis commented Oct 12, 2020

We should support the Cholesky decomposition per https://numpy.org/doc/stable/reference/generated/numpy.linalg.cholesky.html#numpy.linalg.cholesky, adding a pivot argument in the same way as qr and svd.

@alewis alewis added enhancement New feature or request good first issue Good for newcomers labels Oct 12, 2020
@alewis alewis changed the title Add cholesky decomposition Add Cholesky decomposition Oct 12, 2020
@Dbhasin1
Copy link

Hello, can this issue be assigned to me?

@mganahl
Copy link
Collaborator

mganahl commented Oct 25, 2020

Sure thing!

@alewis
Copy link
Collaborator Author

alewis commented Oct 25, 2020

Hi @Dbhasin1,

The function and tests first need to be added to the various backends, which live in tensornetwork/backends/blah. The first is the AbstractBackend, a base class that just throws NotImplementedError. The others all wrap functions from other numerical packages, the idea being to provide a means to quickly switch between these.

Inside the backend files you will see many other functions, such as qr. Our QR differs from e.g. NumPy's by the presence of a pivot argument that matricizes the input. Cholesky should have a similar mechanism.

Once added to the backends, the function should also be added to the tn.Tensor interface, which lives in tensornetwork/tensor.py with functions defined in tensornetwork/linalg/blah

@LuiGiovanni
Copy link
Contributor

Hello there @alewis I'm interested in working on this issue has a PR been done? If not I would like to work on this.

@LuiGiovanni
Copy link
Contributor

Hello, please ignore this PR I made a mistake, the changes implemented for the Cholesky decomposition will be from a different PR.

@LuiGiovanni
Copy link
Contributor

Hello, I am sorry for the mess but #886 is the correct PR I was working on disregard #885 thank you.

@LuiGiovanni
Copy link
Contributor

@alewis @mganahl I have added the NotImplementedError function and it's respective test and made a PR for you to review.

@alewis
Copy link
Collaborator Author

alewis commented Dec 7, 2020

Looks good! Just a small fix.

@LuiGiovanni
Copy link
Contributor

LuiGiovanni commented Dec 7, 2020

@alewis Alright I have made the fix, just waiting on the testing. Also, I will start implementing the Cholesky decomposition, quick question, why does Jax not have it's own decompositions file? Oh, could you add me as an assignee for this issue? Thank you

@mganahl
Copy link
Collaborator

mganahl commented Dec 7, 2020

it uses the numpy decompositions.py file

@LuiGiovanni
Copy link
Contributor

Ok, I see, then I will start adding the function to that file and work my way from there, thank you!

@themnvrao76
Copy link

hey is this still open? can I work on this?

@prat1999
Copy link

Is this issue still open? Can I work on it?

@dhivyasreedhar
Copy link

Hi, can I work on this? I'm new so can someone guide me?

@priorigratia
Copy link

Can I work on this issue?

@ankitasankars
Copy link

Hello, Can I use this issue as my first contribution? I'm new so please can someone guide me?

@mganahl
Copy link
Collaborator

mganahl commented Sep 7, 2021

@prat1999 already opened a PR on this (maybe you guys can work together on this)

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