Implemented Cholesky decomposition to numpy and tensorflow backends by LuiGiovanni · Pull Request #890 · google/TensorNetwork

LuiGiovanni · 2020-12-23T21:39:50Z

Added the decomposition function following the idea of having a pivot on the tensor, this is in reference to issue #852. which we then reshape into a matrix for the Cholesky function used in NumPy linear algebra. which you may find here for reference.

I Will start working on adding the function to the missing backend which is symmetric since Jax uses NumPy's decomposition file. I await any feedback you may have and that you for the help!

…key decomposition.

This reverts commit 81b4d8b.

…i/TensorNetwork into CholeskyDecomposition

…kends, fixing various pylint errors and adding test to each respective backend function

LuiGiovanni · 2020-12-25T20:10:14Z

@alewis @mganahl Hello! I implemented the Cholesky decomposition to various backends following the idea of a pivot value for the tensors, with their respective tests where I defined a matrix that is assured to be positive-definite and also a hermitian matrix:

[1, 0]
[0, 1]

I assumed we won't be validating if the matrices are hermitian or positive-definite since there could be some performance cost in doing so (at least for positive-definite checking) the addition of a hermitian check would be nice to have.

A Pylint error has been occurring in the test runs, but it is somewhere in the code where I haven't made changes to. the file in question is tensornetwork/tn_keras/conv2d_mpo.py which seems to only be affected in versions 3.6 and 3.7, this issue is also occurring in my other PR #889 maybe freezing the version could fix this but this is a lazy solution and would not solve it for all cases, so I will wait for your feedback to see what can be done here. Here is an image of the error for reference.

mganahl · 2020-12-28T10:25:39Z

tensornetwork/backends/numpy/decompositions.py

+
+
+def cholesky(
+    tf: Any,


Rename tf to np (the user passes the numpy module here)

mganahl · 2020-12-28T10:30:56Z

tensornetwork/backends/numpy/decompositions_test.py


+  def test_cholesky(self):
+    #Assured positive-definite hermitian matrixs
+    random_matrix = [[1.0, 0], [0, 1.0]]


Using the identity matrix as "random" matrix is not an ideal test case. You can generate a positive definite matrix e.g. by

A = np.random.rand(D,D) A = A @ A.T.conj()

btw. also test this for all supported dtypes

do np.random.rand(D, D).astype(dtype) to get your fav type. if it's imaginary you might need to initialize the real and imaginary parts separately.

put np.random_seed(10) (or any other fixed integer) before the rand so that the matrix is fixed.

There is no dtype defined in any of the tests, should I define one myself?

mganahl · 2020-12-28T10:36:30Z

tensornetwork/backends/numpy/decompositions.py

+                      [tf.reduce_prod(left_dims),
+                       tf.reduce_prod(right_dims)])
+  L = tf.linalg.cholesky(tensor)
+  if non_negative_diagonal:


This is unnecessary. The cholesky decomposition should already return a lower-triangular matrix with a real, positive diagonal.

mganahl · 2020-12-28T10:37:01Z

tensornetwork/backends/numpy/decompositions.py

+    tf: Any,
+    tensor: Tensor,
+    pivot_axis: int,
+    non_negative_diagonal: bool


remove non_negative argument

(it only makes sense for QR)

mganahl · 2020-12-28T10:37:56Z

tensornetwork/backends/pytorch/decompositions.py

+    torch: Any,
+    tensor: Tensor,
+    pivot_axis: int,
+    non_negative_diagonal: bool = False


remove non_negative_diagonal (see above)

mganahl · 2020-12-28T10:38:28Z

tensornetwork/backends/pytorch/decompositions_test.py


+def test_cholesky():
+  #Assured positive-definite hermitian matrix
+  random_matrix = np.array([[1.0, 0], [0, 1.0]])


use better test matrix (see above)

mganahl · 2020-12-28T10:38:37Z

tensornetwork/backends/pytorch/decompositions_test.py

+  #Assured positive-definite hermitian matrix
+  random_matrix = np.array([[1.0, 0], [0, 1.0]])
+  random_matrix = torch.from_numpy(random_matrix)
+  for non_negative_diagonal in [True, False]:


mganahl · 2020-12-28T10:38:49Z

tensornetwork/backends/tensorflow/decompositions.py

+    tf: Any,
+    tensor: Tensor,
+    pivot_axis: int,
+    non_negative_diagonal: bool


mganahl · 2020-12-28T10:38:56Z

tensornetwork/backends/tensorflow/decompositions.py

+                      [tf.reduce_prod(left_dims),
+                       tf.reduce_prod(right_dims)])
+  L = tf.linalg.cholesky(tensor)
+  if non_negative_diagonal:


mganahl · 2020-12-28T10:39:13Z

tensornetwork/backends/tensorflow/decompositions_test.py


+  def test_cholesky(self):
+    #Assured positive-definite hermitian matrix
+    random_matrix = [[1.0, 0], [0, 1.0]]


modify test, see above

mganahl · 2020-12-28T10:52:20Z

A Pylint error has been occurring in the test runs, but it is somewhere in the code where I haven't made changes to. the file in question is tensornetwork/tn_keras/conv2d_mpo.py which seems to only be affected in versions 3.6 and 3.7, this issue is also occurring in my other PR #889 maybe freezing the version could fix this but this is a lazy solution and would not solve it for all cases, so I will wait for your feedback to see what can be done here. Here is an image of the error for reference.

Thanks for pointing this out. Likely this is due to a new tensorflow release. I'll fix it.

codecov-io · 2020-12-28T11:23:54Z

Codecov Report

Merging #890 (92c931a) into master (f669725) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #890   +/-   ##
=======================================
  Coverage   98.00%   98.00%           
=======================================
  Files         129      129           
  Lines       22625    22665   +40     
=======================================
+ Hits        22173    22213   +40     
  Misses        452      452

Impacted Files	Coverage Δ
tensornetwork/backends/numpy/decompositions.py	`100.00% <100.00%> (ø)`
...ensornetwork/backends/numpy/decompositions_test.py	`98.75% <100.00%> (+0.11%)`	⬆️
tensornetwork/backends/pytorch/decompositions.py	`100.00% <100.00%> (ø)`
...sornetwork/backends/pytorch/decompositions_test.py	`100.00% <100.00%> (ø)`
...ensornetwork/backends/tensorflow/decompositions.py	`100.00% <100.00%> (ø)`
...network/backends/tensorflow/decompositions_test.py	`98.96% <100.00%> (+0.08%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f669725...92c931a. Read the comment docs.

alewis · 2020-12-28T16:40:25Z

tensornetwork/backends/numpy/decompositions.py

+
+
+def cholesky(
+    tf: Any,


alewis · 2020-12-28T16:40:35Z

tensornetwork/backends/numpy/decompositions.py

+    tf: Any,
+    tensor: Tensor,
+    pivot_axis: int,
+    non_negative_diagonal: bool


(it only makes sense for QR)

alewis · 2020-12-28T16:42:25Z

tensornetwork/backends/numpy/decompositions_test.py


+  def test_cholesky(self):
+    #Assured positive-definite hermitian matrixs
+    random_matrix = [[1.0, 0], [0, 1.0]]


do np.random.rand(D, D).astype(dtype) to get your fav type. if it's imaginary you might need to initialize the real and imaginary parts separately.

put np.random_seed(10) (or any other fixed integer) before the rand so that the matrix is fixed.

alewis · 2020-12-28T16:42:59Z

tensornetwork/backends/tensorflow/decompositions.py

+    L = phases[:, None] * L
+  center_dim = tf.shape(L)[1]
+  L = tf.reshape(L, tf.concat([left_dims, [center_dim]], axis=-1))
+  return L


add a newline here (at the end of the file)

…i/TensorNetwork into CholeskyDecomposition

LuiGiovanni · 2020-12-28T21:14:57Z

Hi there @mganahl @alewis In the end I implemented the test following your requested changes and tried these changes out, I'd like your opinion on them. The biggest difference being:

torch.from_numpy(random_ matrix) since it wasn't reading it like a tensor. Let me know what you think. Thank you!

mganahl · 2021-01-04T08:33:50Z

tensornetwork/backends/numpy/decompositions_test.py

+    #Assured positive-definite hermitian matrixs
+    random_matrix = np.random.rand(10, 10)
+    random_matrix = random_matrix @ random_matrix.T.conj()
+    L = decompositions.cholesky(tf, random_matrix, 1)


you are passing the tensorflow module here, but it should be numpy

mganahl · 2021-01-04T08:38:07Z

tensornetwork/backends/numpy/decompositions.py

+  left_dims = np.shape(tensor)[:pivot_axis]
+  right_dims = np.shape(tensor)[pivot_axis:]
+  tensor = np.reshape(tensor,
+                      [np.reduce_prod(left_dims),


np is the numpy module. numpy does not have reduce_prod, that's a tensorflow routine. This only passes the test because in decompositions_test.py you are erroneously passing the tensorflow module instead of the numpy module to numpy.decompositions.cholesky. pls fix this

mganahl · 2021-01-04T08:39:28Z

tensornetwork/backends/pytorch/decompositions.py

+  right_dims = list(tensor.shape)[pivot_axis:]
+
+  tensor = torch.reshape(tensor, (np.prod(left_dims), np.prod(right_dims)))
+  L = np.linalg.cholesky(tensor)


you have to call the torch version of choleksy here, not numpy.

mganahl · 2021-01-18T10:54:22Z

Hi @LuiGiovanni, once the comments are fixed we can merge this

LuiGiovanni and others added 18 commits December 4, 2020 18:14

Added power to Jax backend and test

016d654

Added power function to the Jax backend and test.

62a06c0

Fixed white space error.

43952dd

Re-made the power test function.

b0f579e

Fixed typo

e9f9869

Testing out numpy square.

9e16a9f

Fixed issues with the assertion in test, should work now.

81b4d8b

Added NotImplementedError function and it's respective test for Chols…

195f691

…key decomposition.

Fixed line too long error

737f071

Removing changes for different branches.

0fbe8fb

Revert "Fixed issues with the assertion in test, should work now."

8c37656

This reverts commit 81b4d8b.

Fixed a few pylint issues, should work now

a894960

Fixed typo in the error message

3ba747f

Merge branch 'master' into CholeskyDecomposition

a112ebe

Renamed cholesky function, to a shorter name

f9fb40d

Merge branch 'CholeskyDecomposition' of https://github.com/LuiGiovann…

4b4ece6

…i/TensorNetwork into CholeskyDecomposition

Renamed cholesky function from cholesky_decomposition to cholesky

0c3817d

Implemented Cholesky to numpy and tensorflow with their respective tests

aedfe2a

google-cla bot added the cla: yes label Dec 23, 2020

LuiGiovanni added 3 commits December 23, 2020 15:17

Fixed pylint issues

13026b5

Implemented Cholesky decomposition to tensorflow, numpy & pytorch bac…

11eba6a

…kends, fixing various pylint errors and adding test to each respective backend function

Fixed minor pylint issues

4da6eac

Merge branch 'master' into CholeskyDecomposition

6548989

mganahl suggested changes Dec 28, 2020

View reviewed changes

Merge branch 'master' into CholeskyDecomposition

66b5b94

alewis reviewed Dec 28, 2020

View reviewed changes

requested changes made removed unnecessary arguments and better matrix

dab2060

Merge branch 'CholeskyDecomposition' of https://github.com/LuiGiovann…

e6b9c08

…i/TensorNetwork into CholeskyDecomposition

LuiGiovanni requested a review from mganahl December 28, 2020 18:25

Fixed commented testing functions

367ce3d

Merge branch 'master' into CholeskyDecomposition

92c931a

mganahl suggested changes Jan 4, 2021

View reviewed changes

Merge branch 'master' into CholeskyDecomposition

a51cb9a

Merge branch 'master' into CholeskyDecomposition

2b269e0

Conversation

LuiGiovanni commented Dec 23, 2020

Uh oh!

LuiGiovanni commented Dec 25, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mganahl commented Dec 28, 2020

Uh oh!

codecov-io commented Dec 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LuiGiovanni commented Dec 28, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mganahl commented Jan 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-io commented Dec 28, 2020 •

edited

Loading