Add Linear System Operators to Bonsai.ML.Torch

[ncguilbeault]

This PR adds several new operators to the Bonsai.ML.Torch package for doing operations on linear systems. This includes operators for solving systems with least squares, SVD, multiple matrix multiplications, computing rank, etc.

[glopesdev]

Mostly minor naming and organization comments, otherwise looks great.

[glopesdev]

Thanks, all four of my earlier comments are addressed, and the file-scoped namespace conversion brings the whole LinearAlgebra folder in line with the rest of the project. Builds clean on both target frameworks.

Just one rename is needed for EigenDecompositionResult, and the rest is consistency, all included as inline comments below. A few files are missing the BOM, there are some unused imports, the doc comment style is mixed on occasion, the boolean property descriptions could use the parameter form, and MatrixMultiply diverges from the Concat and Stack precedent.

[glopesdev]

Suggested change

	public readonly struct EigenDecompositionResult((Tensor eigenvalues, Tensor eigenvectors) result)
	public readonly struct EigenvalueDecompositionResult((Tensor eigenvalues, Tensor eigenvectors) result)

Every other result struct mirrors its operator name exactly
(QRDecomposition/QRDecompositionResult, LeastSquares/LeastSquaresResult,
and so on), so this one should be EigenvalueDecompositionResult to match
EigenvalueDecomposition. Worth renaming the file too.

[glopesdev]

Suggested change

	public readonly struct QRDecompositionResult((Tensor Q, Tensor R) result)
	public readonly struct QRDecompositionResult((Tensor q, Tensor r) result)

The tuple element names are capitalized here and camelCase in the other four result structs. I suspect this follows from TorchSharp declaring qr as (Tensor Q, Tensor R), since svd and lstsq are capitalized upstream too but got lowercased here. The tuple is a private implementation detail in a parameter position, so camelCase everywhere is the rule I would pick. The exposed Q and R properties are unaffected.

[glopesdev]

<param name="result"> is empty here and in LeastSquaresResult, SignLogDeterminantResult and SingularValueDecompositionResult, and EigenDecompositionResult is the only one that fills it in. Either fill all five or drop the tag from all five. CS1573 is in NoWarn, so dropping it is fine and is what Concat and Stack already do.

[glopesdev]

Suggested change

	/// The U tensor.
	/// <summary>
	/// Gets the left singular vectors.
	/// </summary>

"The U tensor." and "The Vh tensor." on line 26 do not say what either tensor is, and Vh is the conjugate transpose of V, which is not obvious from the name. QRDecompositionResult says "the orthogonal matrix Q" and "the upper triangular matrix R", which is the form I would follow here. Since the output type is changing, these strings are what someone will read after hitting a build error on Item1, so they are worth filling in.

[glopesdev]

The member docs lead with "The" here and in SingularValueDecompositionResult, but with "Gets the" in EigenDecompositionResult, QRDecompositionResult and SignLogDeterminantResult. These are all get-only properties, so "Gets the" is the form I would standardize on across all five.

[glopesdev]

For a boolean property the [Description] shown in the property grid reads better in the MSDN parameter form, "True to X; otherwise Y.", than with an "Indicates whether" lead, which is XML-doc style aimed at a developer reading IntelliSense rather than at someone toggling the property in the editor. These three are genuine configuration toggles, so:

Suggested change

	[Description("Indicates whether the first matrix is upper triangular.")]
	[Description("True to treat the first matrix as upper triangular; otherwise it is treated as lower triangular.")]

Same for Left and UnitDiagonal below, for Hermitian on MatrixRank.cs:34, and for FullMatrices on SingularValueDecomposition, which currently leads with "Whether". The <summary> can keep "Gets or sets a value indicating whether", and it is worth repeating the "True to X; otherwise Y." instruction in a <remarks> block, since [Description] is not rendered in the API reference.

[ncguilbeault]

I've updated it for this PR but it's worth another PR to update this across the entire codebase.

[glopesdev]

Suggested change

	/// <summary>
	/// The dimension to perform the operation.
	/// </summary>
	/// <summary>
	/// Gets or sets the dimension along which to compute the cross product.
	/// </summary>
	[Description("The dimension along which to compute the cross product.")]

This is the only new property without a [Description], so it will not get a tooltip in the property grid, and the PR adds one to Norm and SingularValueDecomposition. The wording follows Concat.Dimension and Stack.Dimension, which both read "The dimension along which to concatenate the tensors."

The <summary> voice is also mixed across the PR. MatrixRank, QRDecomposition and SolveTriangular lead with "Gets or sets", while this file, Norm, SingularValueDecomposition and TensorSolve lead with "The" or "Whether". "Gets or sets" is the form I would settle on for the summaries. TensorSolve.cs:18 also reads "The dimensions to perform the operation", which is missing a preposition.

[ncguilbeault]

I've updated it for this PR but it's worth another PR to update this across the entire codebase.

[glopesdev]

This file is missing the UTF-8 BOM. The .editorconfig sets charset to utf-8-bom for *.cs, and MatrixMultiply.cs, MatrixRank.cs and TensorSolve.cs are missing it too. The other seven new files have it.

[ncguilbeault]

This should hopefully be fixed

[glopesdev]

Suggested change

	return source.Select(result => new SignLogDeterminantResult(linalg.slogdet(result)));
	return source.Select(tensor => new SignLogDeterminantResult(linalg.slogdet(tensor)));

The lambda parameter is the input tensor, so naming it result reads as the opposite of what it is.

[glopesdev]

Minor, but the folder is now split on this. CrossProduct, Inverse, MatrixMultiply, MatrixRank, QRDecomposition, TensorSolve and this file import static TorchSharp.torch.linalg and call unqualified, while CholeskyDecomposition, Determinant, EigenvalueDecomposition, LeastSquares, Norm, SignLogDeterminant and SingularValueDecomposition qualify with linalg.. LeastSquares and this file were added in the same PR and differ. Either is fine, so it is worth picking one for the folder while all of it is being changed.

[ncguilbeault]

The main issue here is that both TorchSharp and torch have kept identical versions of the same named function in both the torch.* namespace and the torch.linalg.* namespace. The torch.linalg.* version of the operators is the preferred version to use (the deprecated versions now redirect to their linalg cousins), but the C# compiler complains about conflicting functions. I prefer to stick with the the static import of TorchSharp.torch.linalg where possible.

[ncguilbeault]

Thanks for all of your feedback @glopesdev! Hopefully it should be updated.