Cholesky Decomposition added to decompositions.py and decompositions_test.py of both the numpy and tensorflow module.#937
Cholesky Decomposition added to decompositions.py and decompositions_test.py of both the numpy and tensorflow module.#937prat1999 wants to merge 7 commits intogoogle:masterfrom
Conversation
mganahl
left a comment
There was a problem hiding this comment.
thanks for the PR, looks good! After you fixed the comments this should be good to go
| tensor = np.reshape(tensor, [numpy.prod(left_dims), numpy.prod(right_dims)]) | ||
| n = tensor.shape[0] | ||
| m = tensor.shape[1] | ||
| if n != m: |
There was a problem hiding this comment.
Generally its always a good idea to add these types of checks! In this case however I think we can remove them because numpy already performs these kinds of checks for us.
Side note: in most cases, instead of printing a message it's better to raise an exception
| self.assertEqual(L.shape, (3, 1, 3)) | ||
|
|
||
| def test_cholesky(self): | ||
| random_matrix = np.array([[[25, 15, -5]], [[15, 18, 0]], [[-5, 0, 11]]]) |
There was a problem hiding this comment.
not sure if this is random :)
| relative: Optional[bool] = False) -> Tuple[Tensor, Tensor, Tensor, Tensor]: | ||
| """Computes the singular value decomposition (SVD) of a tensor. | ||
|
|
||
There was a problem hiding this comment.
There are a lot of places where you add whitespace like this. Could you remove this from this PR? Thanks
| resulting from combining the axes `tensor.shape[pivot_axis:]`. | ||
|
|
||
| For example, if `tensor` had a shape (2, 3, 4, 5) and `pivot_axis` was 2, | ||
| then `r` would have shape (2, 3, 6), and `q` would |
There was a problem hiding this comment.
replace q and r with L and L_herm
| ```python | ||
| L [i1,...,iN, j] * L_trans[j, k1,...,kM] == tensor[i1,...,iN, k1,...,kM] | ||
| ``` | ||
| Note that the output ordering matches numpy.linalg.svd rather than tf.svd. |
| tf.reduce_prod(right_dims)]) | ||
| n = tensor.shape[0] | ||
| m = tensor.shape[1] | ||
| tensor = tf.cast(tensor, dtype=tf.complex128, name=None) |
There was a problem hiding this comment.
Please remove the cast (is there are reason for this type cast? )
| n = tensor.shape[0] | ||
| m = tensor.shape[1] | ||
| tensor = tf.cast(tensor, dtype=tf.complex128, name=None) | ||
| if n != m: |
There was a problem hiding this comment.
remove checks (tf also performs checks)
Resolved #852 . Cholesky decomposition added to both decompositions.py and decompositions_test.py of the numpy module. Further, it was added to decompositions.py and decompositions_test.py of tensorflow module.