Skip to content

Commit 1273719

Browse files
authored
Merge PR #195: #182 linear-algebra API usable + SVD correctness fix
fix(#182): linear-algebra API was unconstructible and SVD returned wrong results
2 parents 9cd550d + 74f2e47 commit 1273719

7 files changed

Lines changed: 2531 additions & 2426 deletions

File tree

src/Extensions/DotCompute.Algorithms/LinearAlgebra/Components/GpuMatrixOperations.cs

Lines changed: 23 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ namespace DotCompute.Algorithms.LinearAlgebra.Components
1717
/// </summary>
1818
public sealed class GpuMatrixOperations : IDisposable
1919
{
20-
private readonly IKernelManager _kernelManager;
20+
// Optional: GPU kernel execution requires an IKernelManager. No implementation ships yet,
21+
// so this is null in practice and the GPU kernel paths below surface a clear error that
22+
// GPULinearAlgebraProvider catches to fall back to the CPU implementations (GH #182).
23+
private readonly IKernelManager? _kernelManager;
2124
private readonly Dictionary<string, ManagedCompiledKernel> _kernelCache = [];
2225
private bool _disposed;
2326

@@ -26,11 +29,21 @@ public sealed class GpuMatrixOperations : IDisposable
2629
/// </summary>
2730
/// <param name="kernelManager">The kernel manager for compilation and execution.</param>
2831
/// <exception cref="ArgumentNullException">Thrown when kernelManager is null.</exception>
29-
public GpuMatrixOperations(IKernelManager kernelManager)
32+
public GpuMatrixOperations(IKernelManager? kernelManager = null)
3033
{
31-
_kernelManager = kernelManager ?? throw new ArgumentNullException(nameof(kernelManager));
34+
_kernelManager = kernelManager;
3235
}
3336

37+
/// <summary>
38+
/// Returns the kernel manager, or throws a descriptive error when none is available.
39+
/// Callers in <see cref="GPULinearAlgebraProvider"/> catch this and fall back to CPU.
40+
/// </summary>
41+
private IKernelManager RequireKernelManager()
42+
=> _kernelManager ?? throw new InvalidOperationException(
43+
"GPU kernel execution requires an IKernelManager, and no implementation is currently registered. " +
44+
"The CPU implementations (SVD, QR, Cholesky, solvers) work without one — see " +
45+
"src/Extensions/DotCompute.Algorithms/README_LinearAlgebraKernels.md.");
46+
3447
/// <summary>
3548
/// Gets the kernel source for matrix multiply operation.
3649
/// </summary>
@@ -107,7 +120,7 @@ internal async Task<Matrix> MultiplyAsync(Matrix a, Matrix b, IAccelerator accel
107120
var kernel = await GetOrCompileKernelAsync("MatrixMultiply", kernelSource, accelerator, cancellationToken).ConfigureAwait(false);
108121

109122
// Execute the kernel through kernel manager
110-
var executionResult = await _kernelManager.ExecuteKernelAsync(
123+
var executionResult = await RequireKernelManager().ExecuteKernelAsync(
111124
kernel,
112125
arguments,
113126
accelerator,
@@ -217,99 +230,11 @@ internal async Task<Matrix> MultiplyAsync(Matrix a, Matrix b, IAccelerator accel
217230
[System.Diagnostics.CodeAnalysis.SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "Method will use _kernelManager for GPU acceleration in v0.2.1")]
218231
public async Task<(Matrix U, Matrix S, Matrix VT)> SVDAsync(Matrix matrix, IAccelerator accelerator, MatrixProperties properties, HardwareInfo hardware, CancellationToken cancellationToken = default)
219232
{
220-
// GPU-accelerated Jacobi SVD is deferred; this method uses a CPU fallback built
221-
// on the local QR decomposition so the public API works across backends. Consumers
222-
// who need peak GPU SVD performance should integrate cuBLAS directly.
223-
//
224-
// CPU fallback implementation using simplified SVD approach
225-
await Task.Yield(); // Ensure async behavior
226-
227-
var m = matrix.Rows;
228-
var n = matrix.Columns;
229-
230-
// Compute A^T * A for eigenvalue problem
231-
var AtA = new Matrix(n, n);
232-
for (var i = 0; i < n; i++)
233-
{
234-
for (var j = 0; j < n; j++)
235-
{
236-
AtA[i, j] = 0;
237-
for (var k = 0; k < m; k++)
238-
{
239-
AtA[i, j] += matrix[k, i] * matrix[k, j];
240-
}
241-
}
242-
}
243-
244-
// Simplified eigenvalue computation (power iteration for largest eigenvalue)
245-
// This is a basic implementation - production would use QR iteration or Jacobi
246-
var singularValues = new float[Math.Min(m, n)];
247-
var V = new Matrix(n, n);
248-
249-
// Initialize V as identity
250-
for (var i = 0; i < n; i++)
251-
{
252-
V[i, i] = 1.0f;
253-
}
254-
255-
// Extract first singular value (simplified)
256-
if (n > 0 && m > 0)
257-
{
258-
// Power iteration for dominant singular value
259-
var v = new float[n];
260-
for (var i = 0; i < n; i++)
261-
{
262-
v[i] = 1.0f / (float)Math.Sqrt(n);
263-
}
264-
265-
for (var iter = 0; iter < 10; iter++)
266-
{
267-
var Av = new float[n];
268-
for (var i = 0; i < n; i++)
269-
{
270-
Av[i] = 0;
271-
for (var j = 0; j < n; j++)
272-
{
273-
Av[i] += AtA[i, j] * v[j];
274-
}
275-
}
276-
277-
var norm = 0.0f;
278-
for (var i = 0; i < n; i++)
279-
{
280-
norm += Av[i] * Av[i];
281-
}
282-
norm = (float)Math.Sqrt(norm);
283-
284-
if (norm > 1e-10f)
285-
{
286-
for (var i = 0; i < n; i++)
287-
{
288-
v[i] = Av[i] / norm;
289-
}
290-
}
291-
}
292-
293-
singularValues[0] = (float)Math.Sqrt(Math.Max(0, singularValues[0]));
294-
}
295-
296-
// Create S matrix (diagonal with singular values)
297-
var S = new Matrix(m, n);
298-
for (var i = 0; i < Math.Min(m, n); i++)
299-
{
300-
S[i, i] = singularValues[i];
301-
}
302-
303-
// Compute U = A * V * S^-1 (simplified)
304-
var U = new Matrix(m, m);
305-
for (var i = 0; i < m; i++)
306-
{
307-
U[i, i] = 1.0f;
308-
}
309-
310-
var VT = TransposeMatrix(V);
311-
312-
return (U, S, VT);
233+
// GPU-accelerated Jacobi SVD is deferred; use the shared, numerically stable CPU Jacobi
234+
// implementation. The former inline "simplified A^T*A" approach returned unsorted
235+
// singular values and a factorization that did not reconstruct the input (GH #182).
236+
ArgumentNullException.ThrowIfNull(matrix);
237+
return await Task.Run(() => Operations.MatrixDecomposition.ComputeJacobiSVD(matrix), cancellationToken).ConfigureAwait(false);
313238
}
314239

315240
/// <summary>
@@ -347,7 +272,7 @@ private async Task<ManagedCompiledKernel> GetOrCompileKernelAsync(string kernelN
347272

348273
// Compile kernel through kernel manager
349274
// For matrix operations, we use float types for inputs and outputs
350-
var kernel = await _kernelManager.GetOrCompileOperationKernelAsync(
275+
var kernel = await RequireKernelManager().GetOrCompileOperationKernelAsync(
351276
kernelName,
352277
[typeof(float), typeof(float), typeof(float), typeof(int), typeof(int), typeof(int)],
353278
typeof(float),

0 commit comments

Comments
 (0)