Add Linear System Operators to Bonsai.ML.Torch#83
Add Linear System Operators to Bonsai.ML.Torch#83ncguilbeault wants to merge 10 commits intobonsai-rx:mainfrom
Bonsai.ML.Torch#83Conversation
… object instead of static method
glopesdev
changed the title
glopesdev
changed the title
Bonsai.ML.Torch
glopesdev
left a comment
glopesdev
left a comment
There was a problem hiding this comment.
Mostly minor naming and organization comments, otherwise looks great.
| [Combinator] | ||
| [Description("Computes the solution to the least squares and least norm problems for a full rank matrix A of size m*n and a matrix B of size m*k.")] | ||
| [WorkflowElementCategory(ElementCategory.Transform)] | ||
| public class LeastSquaresSolve |
There was a problem hiding this comment.
| public class LeastSquaresSolve | |
| public class LeastSquares |
It feels off to have the verb at the end. I would either move it to the beginning, i.e. SolveLeastSquares, or follow the linalg convention and drop it, i.e. just call this operator LeastSquares.
| /// Represents the result of a QR decomposition. | ||
| /// </summary> | ||
| /// <param name="result"></param> | ||
| public readonly struct QRDecompositionResult((Tensor Q, Tensor R) result) |
There was a problem hiding this comment.
I would move this struct declaration outside the class. That way it would make it easier to manipulate in workflows, visualizers, etc.
However, if it really makes sense to have a nested class, we should remove the QRDecomposition prefix, since its name would be effectively coupled to the name of the container class.
| /// Represents the result of computing the sign and natural logarithm of the absolute value of the determinant. | ||
| /// </summary> | ||
| /// <param name="result"></param> | ||
| public readonly struct SignLogDeterminantResult((Tensor sign, Tensor logabsdet) result) |
There was a problem hiding this comment.
Same comment as above on nested struct classes.
| [Combinator] | ||
| [Description("Computes the solution to a triangular system of linear equations with a unique solution.")] | ||
| [WorkflowElementCategory(ElementCategory.Transform)] | ||
| public class TriangularSolve |
There was a problem hiding this comment.
| public class TriangularSolve | |
| public class SolveTriangular |
I guess now I start to understand where the Solve prefix come from. It looks like linalg itself is not consistent on terminology, but I would still follow their lead for consistency, and rename this class to SolveTriangular.
2 participants
This PR adds several new operators to the
Bonsai.ML.Torchpackage for doing operations on linear systems. This includes operators for solving systems with least squares, SVD, multiple matrix multiplications, computing rank, etc.