[Linear Algebra] Add eye and svd functions

[Shashi456]

So I've not written a test for svd because, I'm not sure how to calculate the permutation for a standard transpose for any given matrix to invoke .tranposed
Since for a tolerance check, i'll have to calculate the reconstructed matrix as u*diag(s) * transpose(v). Will have the test uploaded soon.

I took inspiration for the eye function from here.
The issue with this implementation though is that numCols, batchShape need to be optional, but there is no convenient way for optional parameters in swift. and declaring it as numCols: Int? = nil doesn't exactly compile. So i wanted to ask if i should just overload the function/have multiple function declarations.

Finally, added the setDiagonal function, it's functionality is different from set_matrix_diag of tensorflow because that implementation uses setMatrixDiagV2 which is not available in the rawopsgenerated file.

[Shashi456]

Is there a neat way of doing this in swift:

    m = u.shape[-1]
    n = v.shape[-1]
    if m <= n:
      v = v[..., :m]
    else:
      u = u[..., :n]

[dan-zheng]

Could you please add tests for all newly added operations? (setDiagonal, eye, svd)

[Shashi456]

@dan-zheng could you comment on two things :

1. The optionality of no of columns and no of batches in the eye function
2. If there's a neat way of doing the following with tensors, since array indexing is not very easy. Although i wrote a reconstruction test, it would only work for symmetric matrices

    m = u.shape[-1]
    n = v.shape[-1]
    if m <= n:
      v = v[..., :m]
    else:
      u = u[..., :n]

[dan-zheng]

1. The optionality of no of columns and no of batches in the eye function

Making both column count and batch shape optional sounds good, matching tf.eye:

public func eye<Scalar: Numeric>(
    rowCount: Int,
    columnCount: Int? = nil,
    batchShape: [Int] = []
) {
    let columnCount = columnCount ?? rowCount
    ...
}  

2. If there's a neat way of doing the following with tensors, since array indexing is not very easy. Although i wrote a reconstruction test, it would only work for symmetric matrices

    m = u.shape[-1]
    n = v.shape[-1]
    if m <= n:
      v = v[..., :m]
    else:
      u = u[..., :n]

I believe this is equivalent:

import TensorFlow
var u = Tensor<Float>(ones: [3, 5, 7])
var v = Tensor<Float>(ones: [3, 5])
let m = u.shape.dimensions.last!
let n = v.shape.dimensions.last!
if m <= n {
    v = v[TensorRange.ellipsis, ..<m]
} else {
    u = u[TensorRange.ellipsis, ..<n]
}

[awav]

Can you rename computeUv to computeUV, uv or something more readable? It might follow the language style, but it is slightly obscure, what is meant by computeUv.

[Shashi456]

I think I took this variable name from the raw ops generated. I do agree that it sounds sort of vague.

But would like @dan-zheng's idea on this

[dan-zheng]

I think computeUV is the better name, since Uv is more easily misinterpreted as one camelcased entity than UV. Could you please update the parameter name and doc comments?

[dan-zheng]

A semantic name like computeLeftAndRightSingularVectors may be more idiomatic than computeUV. I'm not sure what's the best semantic name, so using computeUV for now sounds good to me, to be consistent with numpy.svd and tf.linalg.svd.

[Shashi456]

I'll add the tests and other changes today.

[Shashi456]

Will write eye and set diagonal functions next.

[Shashi456]

I'm currently looking at optionality in swift for the eye function, Will write the test as soon as i find a convenient solution.

[Shashi456]

@dan-zheng added the tests and todo for eye function optionality. I'll get around to that soon, but did not want to block this PR.

[SumanSudhir]

Don't you think that it would be better if you write eye function like this
func eye(_ num_rows: Int, _ num_columns: Int = 0)

if we give it input eye(3) or eye(3,3) then it will give the same output
[[1.0, 0.0, 0.0],
[0.0, 1.0, 0.0],
[0.0, 0.0, 1.0]]

but if you give it input as eye(3,2) then output will be
[[1.0, 0.0],
[0.0, 1.0],
[0.0, 0.0]]

There is no batch concept in either numpy or pytorch in eye function but if it is needed then it can be added as.
func eye(_ num_rows: Int, _ num_columns: Int = 0, batch_shape: [Int])
and in a function we can use it as
if(!batch_shape.isEmpty){
}

I have implemented it to contribute to swift and tested it on different test cases but since I am new to open source I don't know how to contribute full function code from scratch.

[Shashi456]

@SumanSudhir the eye function trying to be replicated is that of tensorflow python. I'll take a look at your approach and implement it tomorrow.

[SumanSudhir]

Hey, @Shashi456 if you don't mind then can I pull request for eye function. I have tested it and it is working as expected, it can take input with number of columns and batch shape as an optional parameter and give output in any datatype format as required

Some examples of tested input and output

let x: Tensor<Int32> = eye(4)
[[1, 0, 0, 0],
[0, 1, 0, 0],
[0, 0, 1, 0],
[0, 0, 0, 1]]

let x: Tensor<Int32> = eye(4,5)
[[1, 0, 0, 0, 0],
[0, 1, 0, 0, 0],
[0, 0, 1, 0, 0],
[0, 0, 0, 1, 0]]

let x: Tensor<Int32> = eye(4,batch_shape: [1])
[[[1, 0, 0, 0],
[0, 1, 0, 0],
[0, 0, 1, 0],
[0, 0, 0, 1]]]

let x: Tensor<Int32> = eye(4,5,batch_shape: [1])
[[[1, 0, 0, 0, 0],
[0, 1, 0, 0, 0],
[0, 0, 1, 0, 0],
[0, 0, 0, 1, 0]]]

let x: Tensor<Float32> = eye(4,5,batch_shape: [1])
[[[1.0, 0.0, 0.0, 0.0, 0.0],
[0.0, 1.0, 0.0, 0.0, 0.0],
[0.0, 0.0, 1.0, 0.0, 0.0],
[0.0, 0.0, 0.0, 1.0, 0.0]]]

let x: Tensor<Float32> = eye(4,5,batch_shape: [1,2])
[[[[1.0, 0.0, 0.0, 0.0, 0.0],
[0.0, 1.0, 0.0, 0.0, 0.0],
[0.0, 0.0, 1.0, 0.0, 0.0],
[0.0, 0.0, 0.0, 1.0, 0.0]],

[[1.0, 0.0, 0.0, 0.0, 0.0],
[0.0, 1.0, 0.0, 0.0, 0.0],
[0.0, 0.0, 1.0, 0.0, 0.0],
[0.0, 0.0, 0.0, 1.0, 0.0]]]]

[Shashi456]

@SumanSudhir could you update this function after this PR is merged? That sounds like a better idea.

[Shashi456]

@dan-zheng could you review this once more? Also happy new year :)

[dan-zheng]

Thanks for adding tests for all added operations! I left some more feedback.

[Shashi456]

@dan-zheng sorry for the trivial formatting errors, I'll have the changes up in a few hours.

[Shashi456]

@dan-zheng ping. Built and tested locally.

[dan-zheng]

I left some comments that I'll address personally to save time!

[dan-zheng]

I think computeUV is the better name, since Uv is more easily misinterpreted as one camelcased entity than UV. Could you please update the parameter name and doc comments?

[dan-zheng]

Thank you!

---

It takes a long time to review naming (e.g. argument labels) and API doc comments.

Could you please:

• Follow these naming guidelines.
• Follow these API doc guidelines.
• Follow existing reviewed Swift APIs as a precedent. The API doc comments for func svd are a nice example.

That would be greatly appreciated, and would help accelerate the review process!

[Shashi456]

@dan-zheng, will follow suit for linear algebra functions. Since it was my first time contributing to this area, I wasn't entirely sure of formatting. Sorry!