Skip to content
This repository has been archived by the owner on Nov 7, 2024. It is now read-only.

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
Contributor

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
Contributor

mganahl commented Oct 25, 2020

Sure thing!

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

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
Contributor

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 subscribe to this conversation on GitHub. Already have an account? Sign in.