[Linear Algebra] Add det and slogdet

[Shashi456]

No description provided.

[dan-zheng]

Thanks for adding tests! I left a few minor comments.

[Shashi456]

@dan-zheng I'll take care of the doc comments like you mentioned in the SVD PR. Thanks for the precondition performance explanation.

[dan-zheng]

@dan-zheng I'll take care of the doc comments like you mentioned in the SVD PR. Thanks for the precondition performance explanation.

#578 is actually already merged. The doc comments for Tensor.svd are already polished:

    /// Returns the singular value decomposition of `self`, given that `self` is an optionally
    /// batched matrix.
    /// ...
    func svd(computeUV: Bool = true, fullMatrices: Bool = false) -> ( ... }

[dan-zheng]

Is it possible to define the derivative of slogdet?

---

Motivation: slogdet overlaps with the previously defined func logdet below:

@differentiable(wrt: matrix where T: TensorFlowFloatingPoint)
func logdet<T: TensorFlowFloatingPoint>(_ matrix: Tensor<T>) -> Tensor<T> {
    return 2.0 * log(cholesky(matrix).diagonalPart()).sum(squeezingAxes: -1) 
}

(Aside: func logdet should be renamed to func logDeterminant.)

func logdet is differentiable, while func slogdet is not. If we can define a derivative for func slogdet, we could consider changing the implementation of func logdet to call func slogdet instead of custom computation.

[dan-zheng]

cc @param087, who implemented func logdet in #592.
I wonder what you used as the reference implementation for func logdet? I'm curious why it uses custom logic instead of forwarding to a _Raw op.

[dan-zheng]

@Shashi456: to make progress on this, could you please rename the existing func logdet to func logDeterminant? We can try to unify or code share func logDeterminant and func signAndLogDeterminant in a follow-up.

[awav]

@dan-zheng I would recommend against this renaming. slogdet and logdet are well known and used by every single scientific language I know - Python (numpy), R, Julia, Matlab. The det abbreviation is also broadly used in the literature as well.

[awav]

@Shashi456 ^^^

[Shashi456]

@dan-zheng since I mostly take inspiration from the tensorflow-python API and check numpy linear algebra library for additional consistency. I initially named the functions slogdet and det.

[dan-zheng]

I feel reusing existing term-of-art names here is reasonable. Could you please add (sign:logDeterminant:) result tuple element labels to slogdet please?

func slogdet<T: TensorFlowFloatingPoint>(_ matrix: Tensor<T>) -> (sign: Tensor<T>, logDeterminant: Tensor<T>) {

[param087]

cc @param087, who implemented func logdet in #592.
I wonder what you used as the reference implementation for func logdet? I'm curious why it uses custom logic instead of forwarding to a _Raw op.

@dan-zheng, I had taken reference from https://github.com/tensorflow/tensorflow/blob/r2.1/tensorflow/python/ops/linalg/linalg_impl.py#L72-L103 to implement logdet. Considering the python implementation of logdet and making it differentiable, I found custom logic is more promising than that of forwarding to a _Raw op.

[Shashi456]

@dan-zheng will do so.

[awav]

Continuing my comment #604 (comment),

@dan-zheng, in general, the preference to have short and concise namings for math functions, because often math-like code has long and nested expressions which by itself are really hard to read, https://github.com/GPflow/GPflow/blob/develop/gpflow/expectations/squared_exponentials.py#L79

The name is domain-specific and doesn't make sense to people unfamiliar with numpy.linalg.slogdet or tf.linalg.slogdet

It is not about numpy only, these abbreviations are used everywhere: Python (numpy, tensorflow, pytorch), R, matlab, Julia and just commonly in the literature about linear algebra.

Also, for me renaming of det to determinant looks like renaming log to logarithm.
Please, please, don't do this!

[dan-zheng]

The name is domain-specific and doesn't make sense to people unfamiliar with numpy.linalg.slogdet or tf.linalg.slogdet

It is not about numpy only, these abbreviations are used everywhere: Python (numpy, tensorflow, pytorch), R, matlab, Julia and just commonly in the literature about linear algebra.

Also, for me renaming of det to determinant looks like renaming log to logarithm.
Please, please, don't do this!

Okay, that's fair! For every API, we could try to find an idiomatic Swift name, or to use an existing domain-specific term-of-art name.

In some cases, we chose new idiomatic Swift names:

• Reduction APIs: alongAxes: and squeezingAxes: vs keep_dims=True and keeps_dims=False
• Non-standard APIs: Tensor.bandPart(subdiagonalCount:superdiagonalCount:) instead of argumentless tf.linalg.band_part
  • The meaning of bandPart(1, 2) is completely unclear without reading API documentation. bandPart(subdiagonalCount: 1, superdiagonalCount: 2) is much clearer.

In this case, I feel using an existing term-of-art name is reasonable! det, logdet, and slogdet do seem like standard operations. Let's go with those names for name, but let's still add (sign: Tensor<T>, logDeterminant: Tensor<T>) result tuple element labels for slogdet.

Thanks for your feedback!

[Shashi456]

@dan-zheng WDYT?

[Shashi456]

tests locally pass.

[dan-zheng]

Looks good. Thanks for adding two unit tests for det and slogdet!