chore: phase B GPU: param guards and benchmarks - #676
Conversation
|
Important Review skippedToo many files! 36 files out of 186 files are above the max files limit of 150. You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughImplements a DirectGpu multi-backend GPU stack (CUDA/HIP/OpenCL) with native bindings, DirectGpuEngine and backend factory, kernel fusion and tuned kernels, CLBlast/cuBLAS/cuDNN integrations, GPU memory/timing/health tooling, DenseLayer GPU paths and weight caching, extensive benchmarks, generated tuning DBs, and supporting docs. Changes
Sequence Diagram(s)sequenceDiagram
rect rgb(250,250,245)
actor App
participant Engine as AiDotNet Engine
participant DGE as DirectGpuEngine
participant Factory as DirectGpuBackendFactory
participant Backend as GPU Backend (CUDA/HIP/OpenCL)
participant GPU as GPU Device
end
App->>Engine: CreateOptimalEngine()
Engine->>DGE: Initialize DirectGpuEngine
DGE->>Factory: Detect vendor / available backends
Factory-->>DGE: Selected backend (e.g., CUDA)
DGE->>Backend: Initialize (contexts, streams, kernels)
Backend->>GPU: Allocate memory / compile kernels
App->>DGE: Request MatMul / DenseForwardFused
DGE->>DGE: KernelFusionManager.Check(sequence)
alt Fusion available
DGE->>Backend: Launch fused GEMM+Bias+Activation
else
DGE->>Backend: Launch GEMM
Backend-->>DGE: GEMM result
DGE->>Backend: Launch Activation/Bias kernels
end
Backend->>GPU: Enqueue kernel(s)
GPU-->>Backend: Event (complete)
Backend-->>DGE: Result buffer
DGE-->>Engine: Return result
Engine-->>App: Output
sequenceDiagram
participant Bench as BenchmarkRunner
participant DGE as DirectGpuEngine
participant cuBLAS as CuBlasMatMul (native)
participant ILGPU as ILGPU/Direct kernel
participant GPU as GPU Device
Bench->>DGE: Run MatMul benchmark
alt cuBLAS available & cached weights
DGE->>cuBLAS: MatMulWithCachedWeights(...)
cuBLAS->>GPU: cuBLAS GEMM
cuBLAS-->>DGE: Timings/result
else
DGE->>ILGPU: Launch ILGPU/Direct kernel MatMul(...)
ILGPU->>GPU: Kernel launch
ILGPU-->>DGE: Timings/result
end
DGE-->>Bench: Report GFLOPS & diagnostics (GpuTimingDiagnostics)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
changed the title
|
🤖 PR Title Auto-Fixed Your PR title was automatically updated to follow Conventional Commits format. Original title: New title: Detected type: Valid types and their effects:
If the detected type is incorrect, you can manually edit the PR title. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/AiDotNet.Tensors/Engines/GpuMemoryPool.cs (1)
105-118: Document buffer clearing behavior inRentmethod.The Rent method documentation doesn't mention that returned buffers may or may not be cleared depending on the
clearOnRentsetting used during pool construction. This is important information for callers, as they may need to explicitly clear or initialize buffer contents when clearing is disabled.🔎 Proposed documentation enhancement
/// <summary> /// Rents a GPU memory buffer of at least the specified size. /// </summary> /// <param name="size">The minimum number of elements required.</param> /// <returns>A GPU memory buffer (may be larger than requested).</returns> /// <remarks> /// <para> /// If a buffer is available in the pool, it is reused. Otherwise, a new buffer is allocated. /// The returned buffer may be larger than requested to fit the bucket size. +/// Reused buffers are cleared (zeroed) if the pool was constructed with clearOnRent=true (default). +/// When clearOnRent=false, reused buffers may contain data from previous operations. /// </para> /// <para> /// IMPORTANT: You must call <see cref="Return"/> when done with the buffer to return it to the pool. /// </para> /// </remarks>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
BENCHMARKS.mdsrc/AiDotNet.Tensors/Engines/GpuEngine.cssrc/AiDotNet.Tensors/Engines/GpuMemoryPool.cs
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-18T08:49:25.295Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningMask.cs:1-102
Timestamp: 2025-12-18T08:49:25.295Z
Learning: In the AiDotNet repository, the project-level global using includes AiDotNet.Tensors.LinearAlgebra via AiDotNet.csproj. Therefore, Vector<T>, Matrix<T>, and Tensor<T> are available without per-file using directives. Do not flag missing using directives for these types in any C# files within this project. Apply this guideline broadly to all C# files (not just a single file) to avoid false positives. If a file uses a type from a different namespace not covered by the global using, flag as usual.
Applied to files:
src/AiDotNet.Tensors/Engines/GpuMemoryPool.cssrc/AiDotNet.Tensors/Engines/GpuEngine.cs
📚 Learning: 2025-12-18T08:49:53.103Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningStrategy.cs:1-4
Timestamp: 2025-12-18T08:49:53.103Z
Learning: In this repository, global using directives are declared in AiDotNet.csproj for core namespaces (AiDotNet.Tensors.* and AiDotNet.*) and common system types. When reviewing C# files, assume these global usings are in effect; avoid adding duplicate using statements for these namespaces and for types like Vector<T>, Matrix<T>, Tensor<T>, etc. If a type is not found, verify the global usings or consider adding a file-scoped using if needed. Prefer relying on global usings to reduce boilerplate.
Applied to files:
src/AiDotNet.Tensors/Engines/GpuMemoryPool.cssrc/AiDotNet.Tensors/Engines/GpuEngine.cs
📚 Learning: 2025-11-19T04:08:26.895Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 0
File: :0-0
Timestamp: 2025-11-19T04:08:26.895Z
Learning: For ILGPU GPU operations in GpuEngine.cs, use standard .NET exception types (InvalidOperationException, ArgumentException, OutOfMemoryException) instead of ILGPU-specific exception types, as ILGPU exception types may be version-specific. Combine with message-based filtering using ex.Message.Contains("device") or ex.Message.Contains("accelerator") as a fallback for GPU-specific errors.
Applied to files:
src/AiDotNet.Tensors/Engines/GpuEngine.cs
🧬 Code graph analysis (1)
src/AiDotNet.Tensors/Engines/GpuMemoryPool.cs (1)
src/AiDotNet.Tensors/Engines/GpuEngine.cs (1)
GpuMemoryPool(2006-2013)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Autovalidate
- GitHub Check: CodeQL Analysis
- GitHub Check: commitlint
- GitHub Check: Auto-Fix PR Title
🔇 Additional comments (5)
src/AiDotNet.Tensors/Engines/GpuEngine.cs (3)
1959-1965: LGTM! Well-designed parameter validation helpers.These helper methods provide clean, composable validation for 2D array parameters (stride, padding, dilation). They correctly handle null checks and enable the GPU/CPU fallback logic added throughout this PR.
18108-18108: Excellent optimization: zero-copy data transfer.Replacing
.ToArray()with.AsSpan()eliminates unnecessary heap allocations and provides direct memory access for GPU transfers. This aligns with the PR's zero-copy objective and improves performance for large tensor operations.Also applies to: 18144-18144, 18209-18210, 18255-18256, 22895-22895, 22938-22938, 22992-22992, 23047-23047, 23352-23352, 23397-23397
18290-18296: Well-implemented parameter guards for GPU path.The guards consistently ensure that only symmetric 2D parameters (uniform pairs like
[2,2]) are routed to GPU, while asymmetric parameters (like[2,3]) correctly fall back to CPU. This matches the PR objective and prevents runtime errors from GPU kernels that expect scalar parameters.The two-stage validation (pair validity + uniformity) is clear and handles edge cases properly (null, wrong length, asymmetric values).
Also applies to: 18305-18309, 18421-18425, 18537-18538, 18684-18685, 18835-18836, 18944-18945, 19063-19064, 19178-19179, 19295-19299, 19419-19420, 19536-19537
BENCHMARKS.md (2)
11-13: Verify the dotnet test command and filter syntax.Ensure the run command and filter syntax are correct for the dotnet testing framework. The command should properly invoke the GpuAccelerationBenchmarks test suite.
27-27: No action required. The filedocs/GPU_PERFORMANCE_BENCHMARKS.mdexists in the repository, so the reference in BENCHMARKS.md is valid and not a broken link.
There was a problem hiding this comment.
Pull request overview
This PR implements Phase B GPU enhancements by adding parameter guards to ensure asymmetric 2D array parameters fall back to CPU execution, enabling buffer clearing in the GPU memory pool to prevent data leakage, and documenting the GPU benchmark suite.
Key Changes
- Added parameter validation guards to Conv2D, pooling, and depthwise convolution operations to enforce uniform stride/padding/dilation requirements for GPU execution
- Implemented configurable buffer clearing in GpuMemoryPool to prevent data leakage between buffer reuses (enabled by default)
- Optimized GPU data transfers by replacing ToArray() with AsSpan() calls across multiple operations
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| src/AiDotNet.Tensors/Engines/GpuMemoryPool.cs | Added clearOnRent parameter to constructors and implemented buffer clearing logic to prevent data leakage when reusing pooled GPU buffers |
| src/AiDotNet.Tensors/Engines/GpuEngine.cs | Added helper methods (IsPair, IsUniformPair, IsPositivePair, IsNonNegativePair) and parameter guards across Conv2D, MaxPool2D, AvgPool2D, DepthwiseConv2D, and ConvTranspose2D operations; optimized CPU-to-GPU transfers using AsSpan() |
| BENCHMARKS.md | Added documentation for GPU acceleration benchmarks, including run instructions, expected performance trends, and comparison notes versus PyTorch/TensorFlow |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…eration memory transfers - Add GpuTensorHandle<T> for explicit lifecycle GPU buffer management - Add AllocateGpuTensor methods to GpuEngine for Vector, Matrix, and arrays - Add MatMulDirect, AddDirect, MultiplyDirect operations on GPU handles - Add EnableGpuPersistence to GpuAccelerationConfig (enabled by default) Phase B: US-GPU-030 - Persistent GPU Tensors 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add TensorMatMulCachedWeights methods to GpuEngine for hybrid GPU operations where weights stay on GPU and only input transfers - Add GPU weight caching to DenseLayer with GpuTensorHandle fields and TryTensorMatMulWithCachedWeights helper method - Cache transposed weights to avoid re-transposing every forward pass - Invalidate weight caches when weights change via SetWeights, UpdateParameters, or SetParameters PR #676 comment fixes: - Document security implications of clearOnRent parameter in GpuMemoryPool - Add _accelerator.Synchronize() after MemSetToZero to prevent race conditions - Use IsNonNegativePair for padding validation in Conv2D operations - Update BENCHMARKS.md intro to reference only GPU benchmarks 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
BENCHMARKS.md (1)
3-3: Previous feedback addressed - intro now correctly scopes to GPU benchmarks.The introduction was updated to reference only "GPU benchmark suites," resolving the previous inconsistency.
src/AiDotNet.Tensors/Engines/GpuMemoryPool.cs (2)
66-75: Previous feedback addressed - security implications now documented.The XML documentation for
clearOnRentparameter now includes the security warning as previously suggested:
- Advises to "set to false only in performance-critical scenarios"
- Warns that disabling "may expose sensitive data from previous operations"
- Recommends keeping true for "workloads processing sensitive data"
136-145: Previous feedback addressed - accelerator synchronization added after MemSetToZero.The implementation now correctly calls
_accelerator.Synchronize()afterMemSetToZero()to ensure the asynchronous clear operation completes before returning the buffer. The comment clearly explains the race condition prevention.
🧹 Nitpick comments (4)
AiDotNetBenchmarkTests/Program.cs (1)
17-19: Consider documenting why optimizations validation is disabled.Disabling
DisableOptimizationsValidatorallows benchmarks to run without compiler optimizations, which can produce misleading results. If this is intentional (e.g., for debugging or CI compatibility), consider adding a comment explaining the rationale. If the goal is simply to suppress warnings when running in Debug mode, a better approach might be to ensure benchmarks are always run in Release configuration.🔎 Suggested documentation
var config = ManualConfig.Create(DefaultConfig.Instance) + // Disable optimization validation to allow running benchmarks in environments + // where full optimizations may not be available (e.g., certain CI configurations) .WithOptions(ConfigOptions.DisableOptimizationsValidator);src/AiDotNet.Tensors/Engines/GpuTensorHandle.cs (1)
68-72: Consider returning a defensive copy of Shape to prevent external mutation.The
Shapeproperty returns the internal_shapearray directly, allowing callers to modify it. While this may be intentional for performance, it could lead to subtle bugs if callers inadvertently modify the shape.🔎 Defensive copy option
/// <summary> /// Gets the shape of the tensor (dimensions). /// </summary> - public int[] Shape => _shape; + public int[] Shape => (int[])_shape.Clone();If performance is critical and the shape is expected to be accessed frequently, document that the returned array should not be modified.
AiDotNetBenchmarkTests/TorchSharpComparisonBenchmarks.cs (1)
106-108: Minor formatting issue: closing brace indentation.The closing brace on line 108 appears to be missing indentation. This is a minor cosmetic issue.
🔎 Proposed fix
_torchConvInput?.Dispose(); _torchConvKernel?.Dispose(); -} + }AiDotNetBenchmarkTests/TensorFlowComparisonBenchmarks.cs (1)
93-104: Consider the Conv2D layout implications and remove redundant assignment.The NCHW (AiDotNet) vs NHWC (TensorFlow) layout difference is correctly handled by using separate data arrays. However, the
_tfConvPadding = "SAME"assignment on line 103 is redundant since it's already initialized on line 35.🔎 Proposed fix
_tfConvStrides = new[] { 1, _convStride, _convStride, 1 }; - _tfConvPadding = "SAME"; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
AiDotNetBenchmarkTests/AiDotNetBenchmarkTests.csprojAiDotNetBenchmarkTests/Program.csAiDotNetBenchmarkTests/TensorFlowComparisonBenchmarks.csAiDotNetBenchmarkTests/TorchSharpComparisonBenchmarks.csBENCHMARKS.mddocs/GPU_OPTIMIZATION_REPORT.mdsrc/AiDotNet.Tensors/Engines/GpuEngine.cssrc/AiDotNet.Tensors/Engines/GpuMemoryPool.cssrc/AiDotNet.Tensors/Engines/GpuTensorHandle.cssrc/Engines/GpuAccelerationConfig.cssrc/NeuralNetworks/Layers/DenseLayer.cs
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-12-18T08:49:25.295Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningMask.cs:1-102
Timestamp: 2025-12-18T08:49:25.295Z
Learning: In the AiDotNet repository, the project-level global using includes AiDotNet.Tensors.LinearAlgebra via AiDotNet.csproj. Therefore, Vector<T>, Matrix<T>, and Tensor<T> are available without per-file using directives. Do not flag missing using directives for these types in any C# files within this project. Apply this guideline broadly to all C# files (not just a single file) to avoid false positives. If a file uses a type from a different namespace not covered by the global using, flag as usual.
Applied to files:
src/Engines/GpuAccelerationConfig.cssrc/AiDotNet.Tensors/Engines/GpuMemoryPool.cssrc/NeuralNetworks/Layers/DenseLayer.csAiDotNetBenchmarkTests/Program.csAiDotNetBenchmarkTests/TorchSharpComparisonBenchmarks.cssrc/AiDotNet.Tensors/Engines/GpuTensorHandle.csAiDotNetBenchmarkTests/TensorFlowComparisonBenchmarks.cs
📚 Learning: 2025-12-18T08:49:53.103Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningStrategy.cs:1-4
Timestamp: 2025-12-18T08:49:53.103Z
Learning: In this repository, global using directives are declared in AiDotNet.csproj for core namespaces (AiDotNet.Tensors.* and AiDotNet.*) and common system types. When reviewing C# files, assume these global usings are in effect; avoid adding duplicate using statements for these namespaces and for types like Vector<T>, Matrix<T>, Tensor<T>, etc. If a type is not found, verify the global usings or consider adding a file-scoped using if needed. Prefer relying on global usings to reduce boilerplate.
Applied to files:
src/Engines/GpuAccelerationConfig.cssrc/AiDotNet.Tensors/Engines/GpuMemoryPool.cssrc/NeuralNetworks/Layers/DenseLayer.csAiDotNetBenchmarkTests/Program.csAiDotNetBenchmarkTests/TorchSharpComparisonBenchmarks.cssrc/AiDotNet.Tensors/Engines/GpuTensorHandle.csAiDotNetBenchmarkTests/TensorFlowComparisonBenchmarks.cs
📚 Learning: 2025-12-18T08:49:30.125Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningMask.cs:1-102
Timestamp: 2025-12-18T08:49:30.125Z
Learning: The AiDotNet project uses project-level global usings (configured in AiDotNet.csproj with `<Using Include=AiDotNet.Tensors.LinearAlgebra />`), making Vector<T>, Matrix<T>, and Tensor<T> available in all files without explicit per-file using directives. Do not flag missing using directives for these types in this project.
Applied to files:
AiDotNetBenchmarkTests/AiDotNetBenchmarkTests.csproj
📚 Learning: 2025-12-18T08:50:00.720Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningStrategy.cs:1-4
Timestamp: 2025-12-18T08:50:00.720Z
Learning: The AiDotNet project uses global using directives in src/AiDotNet.csproj via <Using Include="..." /> for AiDotNet.Tensors.LinearAlgebra, AiDotNet.Tensors.Engines, AiDotNet.Tensors.Interfaces, AiDotNet.Tensors.NumericOperations, AiDotNet.Tensors.Helpers, AiDotNet.Autodiff, System.Text, and AiDotNet.Helpers. Types like Vector<T>, Matrix<T>, Tensor<T>, and related linear algebra types are available project-wide without per-file using statements.
Applied to files:
AiDotNetBenchmarkTests/AiDotNetBenchmarkTests.csproj
📚 Learning: 2025-11-19T04:08:26.895Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 0
File: :0-0
Timestamp: 2025-11-19T04:08:26.895Z
Learning: For ILGPU GPU operations in GpuEngine.cs, use standard .NET exception types (InvalidOperationException, ArgumentException, OutOfMemoryException) instead of ILGPU-specific exception types, as ILGPU exception types may be version-specific. Combine with message-based filtering using ex.Message.Contains("device") or ex.Message.Contains("accelerator") as a fallback for GPU-specific errors.
Applied to files:
src/AiDotNet.Tensors/Engines/GpuTensorHandle.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL Analysis
- GitHub Check: Build (Windows)
🔇 Additional comments (19)
src/AiDotNet.Tensors/Engines/GpuTensorHandle.cs (1)
41-89: Well-structured GPU memory handle with proper lifecycle management.The class provides a clean abstraction over ILGPU's
MemoryBuffer1Dwith:
- Proper disposal pattern with finalizer safety net
- Dirty tracking for lazy CPU synchronization
- Clear input validation in constructor
The internal constructor appropriately restricts instantiation to trusted callers (GpuEngine).
src/Engines/GpuAccelerationConfig.cs (1)
167-191: Excellent documentation for the GPU persistence feature.The XML documentation thoroughly covers:
- The purpose and performance benefits (up to 100x speedups)
- When to disable (memory constraints, debugging)
- Memory impact with concrete examples (100M params = 400MB)
The default of
trueis appropriate given the significant performance benefits.docs/GPU_OPTIMIZATION_REPORT.md (1)
1-62: Valuable and transparent performance documentation.This report provides:
- Honest assessment of current GPU performance gaps
- Clear identification of bottlenecks with code evidence
- Well-prioritized optimization roadmap (P0/P1/P2)
- Concrete validation plan
The transparency about AiDotNet GPU being slower than CPU-based competitors is helpful for users making informed decisions and for tracking future improvements.
src/NeuralNetworks/Layers/DenseLayer.cs (2)
643-696: GPU matmul caching logic is well-structured with proper fallback.The implementation correctly:
- Checks for GpuEngine availability before attempting GPU operations
- Lazily allocates GPU handles on first use
- Returns
nullto trigger CPU fallback on any failure- Handles both
floatanddoubletypes separatelyThe null-coalescing fallback pattern on line 808 (
matmul ??= Engine.TensorMatMul(...)) ensures robustness.
599-602: Cache invalidation correctly wired to weight modification points.
InvalidateWeightCaches()is called from:
SetWeights()(line 601)UpdateParameters()(line 1145)SetParameters()(line 1230)This ensures the GPU cache stays synchronized with weight changes.
src/AiDotNet.Tensors/Engines/GpuMemoryPool.cs (1)
86-98: Robust bucket size validation prevents invalid configurations.The validation loop correctly ensures:
- All bucket sizes are positive
- Bucket sizes are strictly ascending
Error messages include specific indices and values for debugging.
BENCHMARKS.md (1)
94-94: Verify referenced documentation file exists.Line 94 references
docs/GPU_PERFORMANCE_BENCHMARKS.md, but this file doesn't appear in the PR files. Ensure this file exists or update the reference.AiDotNetBenchmarkTests/AiDotNetBenchmarkTests.csproj (1)
25-26: Add platform-conditional references for Linux CUDA support if benchmarks run on non-Windows CI.
TorchSharp-cuda-windowsis Windows-specific. If benchmarks run on Linux CI, CUDA operations will fail. TorchSharp-cuda-linux (v0.105.2) is available on NuGet. Consider using project-level conditionals to reference the correct platform-specific CUDA package, or useTorchSharp-cpuas a cross-platform fallback:<PackageReference Include="TorchSharp-cuda-windows" Version="0.105.2" Condition="'$(TargetFramework)' == 'net8.0' AND '$(OS)' == 'Windows_NT'" /> <PackageReference Include="TorchSharp-cuda-linux" Version="0.105.2" Condition="'$(TargetFramework)' == 'net8.0' AND '$(OS)' != 'Windows_NT'" />AiDotNetBenchmarkTests/TorchSharpComparisonBenchmarks.cs (7)
11-42: LGTM!The benchmark configuration and field organization are well-structured. Using dictionaries keyed by size for both frameworks enables parameterized benchmarks effectively.
44-81: LGTM!Good setup practices: disabling gradients for benchmarking, detecting CUDA availability, and using deterministic data for reproducibility. The warmup call helps eliminate first-run overhead from timing measurements.
110-130: LGTM!The warmup sequence appropriately exercises key operations on both frameworks to eliminate first-run overhead from benchmark measurements. Proper disposal of temporary results prevents resource leaks.
132-157: LGTM!Conv2D initialization uses consistent NCHW layout and identical data for both frameworks, ensuring a fair comparison.
159-170: LGTM!The
ConsumeTorchResultmethod correctly handles GPU synchronization by copying CUDA results to CPU, ensuring the benchmark timing includes complete GPU execution.
172-190: LGTM!The deterministic data generation using an LCG-style formula ensures reproducible benchmarks across runs.
192-307: LGTM!The benchmark methods are well-structured with proper resource disposal for TorchSharp tensors. The different return patterns (returning value vs. void with Consumer) are both valid approaches to prevent dead-code elimination in BenchmarkDotNet.
AiDotNetBenchmarkTests/TensorFlowComparisonBenchmarks.cs (4)
9-35: LGTM!Field organization mirrors the TorchSharp benchmark class structure appropriately.
37-75: LGTM!Setup correctly initializes tensors for both frameworks with deterministic data and includes warmup calls to eliminate first-run overhead.
106-124: LGTM!The deterministic data generation matches the TorchSharp benchmark implementation, ensuring consistent test data across comparison suites.
180-248: LGTM!The remaining benchmark methods follow a consistent pattern. The
.numpy()calls ensure GPU synchronization before timing ends, making the comparison fair.
|
- Fix integer overflow in TensorMatMul threshold check (m*n*p > int.MaxValue) caused GPU path to be silently skipped for matrices >= 2048x2048 - Add transpose-B optimization for large matrices (k >= 2048) to avoid catastrophic cache misses when accessing columns of matrix B - Add GpuTimingDiagnostics for performance profiling - Add GPU_MATMUL_ANALYSIS.md documenting the root cause and fix Before: 2048x2048 matmul = 0.13 GFLOPS (fell back to CPU due to overflow) After: 2048x2048 matmul = 51.90 GFLOPS (400x improvement) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/AiDotNet.Tensors/Engines/GpuTimingDiagnostics.cs (2)
99-106: Consider validatingmaxRecordsparameter.If
maxRecordsis passed as0or negative, the trimming logic inRecord()(line 129) would enter an infinite loop attempting to trim_recordsbelow an unreachable target.🔎 Proposed validation
public GpuTimingDiagnostics(bool enabled = false, int maxRecords = 10000) { + if (maxRecords <= 0) + throw new ArgumentOutOfRangeException(nameof(maxRecords), "maxRecords must be positive."); + _isEnabled = enabled; _maxRecords = maxRecords; _records = new ConcurrentQueue<GpuTimingRecord>(); _operationCounts = new ConcurrentDictionary<string, long>(); _operationTotalMs = new ConcurrentDictionary<string, double>(); }
116-130: Aggregate dictionaries grow unbounded.While
_recordsis trimmed to_maxRecords, the_operationCountsand_operationTotalMsdictionaries accumulate entries for every uniqueoperation:categorykey without limit. If many distinct operations are recorded over time, these dictionaries could grow large.For a diagnostics tool this is likely acceptable, but consider whether
Clear()should be called periodically, or document this behavior.GPU_MATMUL_ANALYSIS.md (1)
38-45: Add blank lines around tables for consistent Markdown rendering.Markdown linters expect tables to be surrounded by blank lines. This applies to both tables in the document.
🔎 Proposed fix
### Scaling Analysis + | Size | Stride | Expected | Observed | |------|--------|----------|----------| | 256x256 | 256 | OK | 23 GFLOPS | | 512x512 | 512 | OK | 67 GFLOPS | | 1024x1024 | 1024 | Degraded | 84 GFLOPS | | 2048x2048 | 2048 | **Catastrophic** | 0.13 GFLOPS | +Similarly for the results table at lines 80-87.
src/AiDotNet.Tensors/Engines/GpuEngine.cs (2)
2488-2493: TODO flagged for tiled matrix multiply optimization.The comment acknowledges the limitation of the current naive kernel approach and identifies the path forward for 2D tiled optimization.
Do you want me to open a tracking issue for implementing the 2D tiled kernel when ILGPU adds support or via a custom kernel launcher?
18251-18256: Consider extracting TransposeThreshold as a class-level constant.The threshold value 2048 is duplicated in both float (line 18255) and double (line 18371) matrix multiply implementations. Extracting it as a private const field would improve maintainability if this threshold needs adjustment.
🔎 Proposed refactoring
Add at class level near other constants:
+ // Threshold for switching to transpose-B optimization in matrix multiply + // For k >= this value, the cache miss penalty outweighs transpose overhead + private const int MatMulTransposeThreshold = 2048;Then reference it in both methods:
- const int TransposeThreshold = 2048; - bool useOptimizedKernel = k >= TransposeThreshold && _matrixMultiplyTransposedBKernelFloat != null; + bool useOptimizedKernel = k >= MatMulTransposeThreshold && _matrixMultiplyTransposedBKernelFloat != null;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
GPU_MATMUL_ANALYSIS.mdsrc/AiDotNet.Tensors/Engines/GpuEngine.cssrc/AiDotNet.Tensors/Engines/GpuTimingDiagnostics.cs
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-19T04:08:26.895Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 0
File: :0-0
Timestamp: 2025-11-19T04:08:26.895Z
Learning: For ILGPU GPU operations in GpuEngine.cs, use standard .NET exception types (InvalidOperationException, ArgumentException, OutOfMemoryException) instead of ILGPU-specific exception types, as ILGPU exception types may be version-specific. Combine with message-based filtering using ex.Message.Contains("device") or ex.Message.Contains("accelerator") as a fallback for GPU-specific errors.
Applied to files:
src/AiDotNet.Tensors/Engines/GpuEngine.cssrc/AiDotNet.Tensors/Engines/GpuTimingDiagnostics.cs
📚 Learning: 2025-12-18T08:49:25.295Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningMask.cs:1-102
Timestamp: 2025-12-18T08:49:25.295Z
Learning: In the AiDotNet repository, the project-level global using includes AiDotNet.Tensors.LinearAlgebra via AiDotNet.csproj. Therefore, Vector<T>, Matrix<T>, and Tensor<T> are available without per-file using directives. Do not flag missing using directives for these types in any C# files within this project. Apply this guideline broadly to all C# files (not just a single file) to avoid false positives. If a file uses a type from a different namespace not covered by the global using, flag as usual.
Applied to files:
src/AiDotNet.Tensors/Engines/GpuEngine.cssrc/AiDotNet.Tensors/Engines/GpuTimingDiagnostics.cs
📚 Learning: 2025-12-18T08:49:53.103Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningStrategy.cs:1-4
Timestamp: 2025-12-18T08:49:53.103Z
Learning: In this repository, global using directives are declared in AiDotNet.csproj for core namespaces (AiDotNet.Tensors.* and AiDotNet.*) and common system types. When reviewing C# files, assume these global usings are in effect; avoid adding duplicate using statements for these namespaces and for types like Vector<T>, Matrix<T>, Tensor<T>, etc. If a type is not found, verify the global usings or consider adding a file-scoped using if needed. Prefer relying on global usings to reduce boilerplate.
Applied to files:
src/AiDotNet.Tensors/Engines/GpuEngine.cssrc/AiDotNet.Tensors/Engines/GpuTimingDiagnostics.cs
🧬 Code graph analysis (1)
src/AiDotNet.Tensors/Engines/GpuTimingDiagnostics.cs (2)
src/Helpers/InferenceDiagnostics.cs (1)
IsEnabled(16-21)src/AiDotNet.Tensors/Engines/Optimization/PerformanceProfiler.cs (1)
IDisposable(38-44)
🪛 markdownlint-cli2 (0.18.1)
GPU_MATMUL_ANALYSIS.md
39-39: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL Analysis
- GitHub Check: Build (Windows)
🔇 Additional comments (17)
src/AiDotNet.Tensors/Engines/GpuTimingDiagnostics.cs (3)
10-33: LGTM!The timing categories are well-chosen and provide comprehensive coverage of GPU operation phases relevant for profiling.
38-59: LGTM!The readonly struct is appropriate for immutable timing records, and using
Stopwatch.GetTimestamp()provides high-resolution timing.
261-286: Well-designed RAII timing scope.The
TimingScopereadonly struct pattern avoids heap allocations and correctly short-circuits when diagnostics are disabled. The check at both construction (line 275) and disposal (line 280) ensures minimal overhead in production.GPU_MATMUL_ANALYSIS.md (2)
1-37: Excellent root cause documentation.The analysis clearly explains the cache miss problem with concrete numbers. The stride-based memory access pattern explanation will help future maintainers understand why the transpose optimization was necessary.
66-107: Thorough implementation documentation.The documentation of the integer overflow bug (line 71-73) is particularly valuable—this silent CPU fallback would have been difficult to diagnose. The threshold rationale (k >= 2048) is well-justified by the benchmark data.
src/AiDotNet.Tensors/Engines/GpuEngine.cs (12)
1529-1530: LGTM!The timing diagnostics field is properly declared as readonly and follows the established patterns in the codebase.
1622-1625: LGTM!The optimized matrix multiply kernel declarations are well-documented and the comment clearly explains the cache optimization strategy.
1967-1974: LGTM!The
TimingDiagnosticsproperty is well-documented and correctly exposes the diagnostics functionality to consumers while emphasizing its opt-in nature.
2042-2066: LGTM!The constructor changes maintain backward compatibility while adding opt-in timing diagnostics. The default of
falseis appropriate for production scenarios.
2405-2418: LGTM!The optimized matrix multiply kernel correctly implements the transpose-B strategy for coalesced memory access, addressing the cache miss issues documented in the PR objectives.
2506-2515: LGTM!The double-precision version of the optimized kernel mirrors the float implementation correctly.
18233-18234: Critical fix for large matrix support.The change from
inttolongfortotalOpscorrectly prevents integer overflow for large matrices (≥2048×2048), restoring the GPU execution path as documented in the commit message.
18258-18290: LGTM!The timing instrumentation is well-designed:
- Conditional stopwatch creation minimizes overhead when disabled
- Synchronization for accurate timing is only performed when measuring
- Timing records include byte counts for bandwidth analysis
18297-18331: LGTM with note on serialization.The kernel execution logic correctly:
- Conditionally transposes B for large matrices
- Selects the appropriate kernel based on matrix size
- Synchronizes to ensure kernel completion before unlocking
The lock serializes all GPU operations, which is necessary for thread safety with shared accelerator and memory pool resources. This is appropriate for Phase B.
18354-18357: LGTM!The conditional return of
gpuBTcorrectly handles the case where the optimized kernel was not used and the buffer was not allocated.
18367-18441: LGTM!The double-precision matrix multiply implementation correctly mirrors the float version with appropriate type changes. All the optimization strategies (transpose-B for large k, adaptive kernel selection, comprehensive timing) are consistently applied.
18470-18473: LGTM!The cleanup pattern is consistent with the float version and correctly handles the conditional
gpuBTbuffer.
MemSetToZero is asynchronous by default on ILGPU's default stream. Add explicit Accelerator.Synchronize() call to ensure the buffer is zeroed before setting the dirty flag, preventing potential race conditions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…tmul Phase B: US-GPU-030 - Persistent GPU Tensors - Add IDisposable to LayerBase for proper GPU resource cleanup - Implement AllocateGpuTensor for persistent (non-pooled) GPU storage - Implement TensorMatMulCachedWeights for weight-cached matrix multiply - Update DenseLayer to use persistent GPU weights across forward passes - Add proper Dispose pattern with base class override in DenseLayer Performance Impact: - Without caching: 864 transfers for 36 forward passes - With caching: 2 transfers (initial upload, final download) - Expected speedup: 100-1000x for weight-heavy operations 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Phase B: GPU Performance Optimization Root cause analysis of 1024->2048 performance regression: - Naive kernel loads 2K values per output element from global memory - For 2048x2048: 4M elements × 4096 loads = 16B memory accesses - Memory bandwidth becomes bottleneck, not compute Tiled kernel infrastructure added: - TiledMatMulParams struct for matrix dimensions - TiledMatMulKernels static class with 32x32 tile size - Shared memory allocation pattern for A and B tiles - Group.Barrier() synchronization for tile loading Note: Tiled kernel loading requires ILGPU's explicitly grouped kernel API with SharedMemory.Allocate2D. Current implementation uses transpose-B optimization which provides 10-50x speedup but tiled kernels would provide an additional 2-10x for k >= 2048. Expected performance after tiled implementation: - 256x256: ~50 GFLOPS (current: 22) - 512x512: ~120 GFLOPS (current: 65) - 1024x1024: ~180 GFLOPS (current: 86) - 2048x2048: ~200 GFLOPS (current: 52) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/AiDotNet.Tensors/Engines/GpuEngine.cs (1)
18712-18721: Add rank validation before accessing Shape indices.Same issue as the float version: accessing
Shape[0]andShape[1]without validating tensor rank will cause confusing errors for non-2D tensors.🔎 Suggested validation
+ if (input.Shape.Length != 2) + throw new ArgumentException($"Input must be 2D tensor, got {input.Shape.Length}D", nameof(input)); + if (weights.Shape.Length != 2) + throw new ArgumentException($"Weights must be 2D tensor, got {weights.Shape.Length}D", nameof(weights)); + if (gpuWeights.Shape.Length != 2) + throw new ArgumentException($"GPU weights must be 2D tensor, got {gpuWeights.Shape.Length}D", nameof(gpuWeights)); + // Input: [batch, inputFeatures], WeightsT: [outputFeatures, inputFeatures] // Result: [batch, outputFeatures] int m = input.Shape[0]; // batch size
🧹 Nitpick comments (4)
src/AiDotNet.Tensors/Engines/GpuTensorHandle.cs (4)
56-61: Verify View properties don't throw on disposed buffers.The
View1DandViewproperties access_buffer.Viewwithout checking_disposed. If the buffer is disposed, ILGPU may return invalid views or throw exceptions with unclear messages.🔎 Consider adding disposal checks
- public ArrayView1D<T, Stride1D.Dense> View1D => _buffer.View; + public ArrayView1D<T, Stride1D.Dense> View1D + { + get + { + ThrowIfDisposed(); + return _buffer.View; + } + } - public ArrayView<T> View => _buffer.View.BaseView; + public ArrayView<T> View + { + get + { + ThrowIfDisposed(); + return _buffer.View.BaseView; + } + }
68-71: Shape array is exposed directly - consider defensive copy.The
Shapeproperty returns the internal_shapearray directly, allowing callers to mutate it. For a low-level GPU handle this may be acceptable for performance, but could cause subtle bugs.
104-107: ClearDirty lacks disposal check.The
ClearDirtymethod doesn't callThrowIfDisposed(). While it's internal, consistency with other methods would improve robustness.🔎 Proposed fix
internal void ClearDirty() { + ThrowIfDisposed(); _gpuDirty = false; }
191-208: Finalizer swallows exceptions appropriately but consider logging.The finalizer correctly catches and swallows exceptions (as required for finalizers), but consider logging a warning in debug builds when disposal happens via finalizer rather than explicit
Dispose(). Based on learnings, use standard .NET exception types.🔎 Optional: Add debug logging for finalizer path
~GpuTensorHandle() { // Attempt cleanup if Dispose wasn't called // Note: This runs on finalizer thread, GPU operations may not be safe // This is a safety net, not the recommended disposal pattern if (!_disposed) { +#if DEBUG + System.Diagnostics.Debug.WriteLine( + $"Warning: GpuTensorHandle<{typeof(T).Name}> was not disposed. " + + "Consider using 'using' statement or calling Dispose() explicitly."); +#endif try { _buffer.Dispose(); } catch { // Swallow exceptions in finalizer } _disposed = true; } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/AiDotNet.Tensors/Engines/GpuEngine.cssrc/AiDotNet.Tensors/Engines/GpuTensorHandle.cssrc/NeuralNetworks/Layers/DenseLayer.cssrc/NeuralNetworks/Layers/LayerBase.cs
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: ooples
Repo: ooples/AiDotNet PR: 0
File: :0-0
Timestamp: 2025-11-19T04:08:26.895Z
Learning: For ILGPU GPU operations in GpuEngine.cs, use standard .NET exception types (InvalidOperationException, ArgumentException, OutOfMemoryException) instead of ILGPU-specific exception types, as ILGPU exception types may be version-specific. Combine with message-based filtering using ex.Message.Contains("device") or ex.Message.Contains("accelerator") as a fallback for GPU-specific errors.
📚 Learning: 2025-11-19T04:08:26.895Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 0
File: :0-0
Timestamp: 2025-11-19T04:08:26.895Z
Learning: For ILGPU GPU operations in GpuEngine.cs, use standard .NET exception types (InvalidOperationException, ArgumentException, OutOfMemoryException) instead of ILGPU-specific exception types, as ILGPU exception types may be version-specific. Combine with message-based filtering using ex.Message.Contains("device") or ex.Message.Contains("accelerator") as a fallback for GPU-specific errors.
Applied to files:
src/AiDotNet.Tensors/Engines/GpuEngine.cssrc/AiDotNet.Tensors/Engines/GpuTensorHandle.cs
📚 Learning: 2025-12-18T08:49:25.295Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningMask.cs:1-102
Timestamp: 2025-12-18T08:49:25.295Z
Learning: In the AiDotNet repository, the project-level global using includes AiDotNet.Tensors.LinearAlgebra via AiDotNet.csproj. Therefore, Vector<T>, Matrix<T>, and Tensor<T> are available without per-file using directives. Do not flag missing using directives for these types in any C# files within this project. Apply this guideline broadly to all C# files (not just a single file) to avoid false positives. If a file uses a type from a different namespace not covered by the global using, flag as usual.
Applied to files:
src/AiDotNet.Tensors/Engines/GpuEngine.cssrc/NeuralNetworks/Layers/DenseLayer.cssrc/AiDotNet.Tensors/Engines/GpuTensorHandle.cssrc/NeuralNetworks/Layers/LayerBase.cs
📚 Learning: 2025-12-18T08:49:53.103Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningStrategy.cs:1-4
Timestamp: 2025-12-18T08:49:53.103Z
Learning: In this repository, global using directives are declared in AiDotNet.csproj for core namespaces (AiDotNet.Tensors.* and AiDotNet.*) and common system types. When reviewing C# files, assume these global usings are in effect; avoid adding duplicate using statements for these namespaces and for types like Vector<T>, Matrix<T>, Tensor<T>, etc. If a type is not found, verify the global usings or consider adding a file-scoped using if needed. Prefer relying on global usings to reduce boilerplate.
Applied to files:
src/AiDotNet.Tensors/Engines/GpuEngine.cssrc/NeuralNetworks/Layers/DenseLayer.cssrc/AiDotNet.Tensors/Engines/GpuTensorHandle.cssrc/NeuralNetworks/Layers/LayerBase.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build (Windows)
- GitHub Check: CodeQL Analysis
🔇 Additional comments (29)
src/AiDotNet.Tensors/Engines/GpuTensorHandle.cs (4)
41-46: Well-structured class with proper generic constraints.The class design properly constrains
Ttounmanagedtypes as required by ILGPU. The internal fields appropriately track disposal state and GPU dirty flag.
83-89: Constructor properly validates inputs.Null checks and field initialization are correct. Setting
_gpuDirty = falseis appropriate since no GPU operations have modified the buffer yet.
142-152: CopyFromCpu clears dirty flag but data was just written to GPU.After
CopyFromCpu, the GPU buffer contains new data from CPU. Setting_gpuDirty = falseis correct since CPU and GPU are now synchronized, but the semantics feel inverted. The dirty flag tracks "GPU modified, CPU stale" - after a CPU write, this is indeed false.
154-168: Synchronization after MemSetToZero correctly addresses async behavior.The explicit
Synchronize()call afterMemSetToZero()ensures the buffer is actually zeroed before the dirty flag is set. This correctly addresses the ILGPU async behavior noted in past reviews.src/NeuralNetworks/Layers/DenseLayer.cs (9)
236-244: GPU weight caching fields properly conditionally compiled.The GPU-specific fields are correctly gated behind
#if !NET462. Maintaining separate handles for float and double is the right approach sinceGpuTensorHandle<T>is generic but requires concrete type specialization for GPU operations.
609-621: Cache invalidation correctly disposes GPU handles.The method properly disposes GPU handles before nulling them, preventing GPU memory leaks. The null-conditional disposal pattern (
?.Dispose()) is idiomatic and safe.
627-634: Transposed weights cache is efficient and correct.Lazy caching of transposed weights avoids redundant transpose operations on every forward pass. This is a good optimization.
657-709: GPU-cached matmul implementation is well-structured.The method correctly:
- Checks for GPU engine availability
- Lazily allocates GPU handles on first use
- Falls back gracefully by returning
null- Handles both float and double types
However, the pattern of checking
typeof(T) == typeof(float)and then casting could benefit from a comment explaining why generic constraints don't suffice here.
810-820: Forward pass correctly tries GPU path first with CPU fallback.The null-coalescing pattern
matmul ??= Engine.TensorMatMul(...)elegantly handles GPU failure by falling back to CPU. The cached transposed weights are used in both paths.
1173-1176: Cache invalidation on parameter update is correct.Invalidating caches after weight updates ensures the GPU-cached transposed weights remain consistent with the updated CPU weights.
1258-1260: Cache invalidation on SetParameters is correct.This ensures consistency when parameters are set externally (e.g., loading from checkpoint).
1473-1488: Dispose pattern correctly releases GPU resources.The override properly:
- Calls
InvalidateWeightCaches()to dispose GPU handles- Clears managed state references
- Calls
base.Dispose(disposing)This addresses the past review comment about implementing IDisposable.
676-683: Potential issue: input tensors re-uploaded every call despite weight caching.The method uploads
inputTensorto GPU every forward pass viaTensorMatMulCachedWeights. While weights are cached (the main goal), verify thatTensorMatMulCachedWeightsefficiently handles the input tensor. The comment on lines 652-654 indicates this is the expected behavior, but consider documenting the transfer cost.src/NeuralNetworks/Layers/LayerBase.cs (4)
264-267: Disposal tracking field correctly placed.The
_disposedfield is appropriately private and placed near the related disposal logic section.
1806-1810: Dispose implementation follows recommended pattern.The public
Dispose()correctly:
- Calls
Dispose(true)- Calls
GC.SuppressFinalize(this)This is the standard C# disposal pattern.
1837-1849: Base Dispose(bool) provides proper template for derived classes.The empty
if (disposing)block is appropriate for the base class since it has no managed resources. The documentation clearly instructs derived classes to override and callbase.Dispose(disposing).
1854-1857: Finalizer correctly calls Dispose(false).The finalizer follows the pattern of calling
Dispose(false)to indicate finalization context. This ensures derived classes can differentiate between explicit disposal and finalization.src/AiDotNet.Tensors/Engines/GpuEngine.cs (12)
1529-1530: LGTM! Timing diagnostics infrastructure added.The timing diagnostics field is properly declared and will enable performance profiling when explicitly enabled.
1622-1625: LGTM! Optimized kernels for large matrix operations.The transposed-B kernels address cache miss penalties for large matrices. The comments clearly explain the optimization strategy.
1967-1974: LGTM! Well-documented public API for diagnostics.The property provides access to timing diagnostics with clear documentation about the opt-in nature and performance implications.
2042-2065: LGTM! Constructor changes maintain backward compatibility.The timing diagnostics parameter is opt-in with good defaults, and the initialization is correct.
2405-2418: LGTM! Optimized kernel compiled with correct memory access pattern.The transposed-B kernel ensures coalesced memory access by reading rows instead of columns, avoiding strided access penalties.
2488-2493: Acknowledged: Tiled kernels deferred for future work.The TODO is reasonable given ILGPU's current grouping limitations. The naive kernel with optimized memory access is a solid interim solution.
2506-2515: LGTM! Double precision optimized kernel matches float implementation.The kernel logic and memory access patterns are correct.
18233-18234: LGTM! Overflow prevention for large matrix operations.Using
longprevents overflow for large matrices (e.g., 2048³ > int.MaxValue).
18251-18290: LGTM! Adaptive kernel selection with comprehensive timing.The threshold-based selection between naive and optimized kernels is well-reasoned, and the timing instrumentation will help validate the trade-offs.
18296-18342: LGTM! Correct kernel execution and efficient result handling.The conditional kernel selection logic is sound, and using SubView properly handles pooled buffers that may be larger than needed.
18354-18357: LGTM! Proper cleanup of optional transpose buffer.The conditional return prevents double-free and ensures resources are released.
18367-18456: LGTM! Double precision implementation consistent with float version.The adaptive kernel selection and timing logic match the float implementation correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
src/AiDotNet.Tensors/Engines/GpuEngine.cs (4)
18690-18694: Integer overflow risk in shape validation.The loop multiplies dimensions without overflow protection. For very large shapes,
expectedLengthcould overflow and wrap around, causing incorrect validation. This is the same issue flagged in past review comments at lines 18508-18512.🔎 Suggested fix with overflow checking
int expectedLength = 1; foreach (int dim in shape) +{ + if (expectedLength > int.MaxValue / dim) + throw new ArgumentException($"Shape dimensions too large: {string.Join("x", shape)} would overflow"); expectedLength *= dim; +} if (data.Length != expectedLength) throw new ArgumentException($"Data length {data.Length} doesn't match shape {string.Join("x", shape)} = {expectedLength}");
18729-18733: Integer overflow risk in shape validation (double overload).Same overflow issue as the float overload. Apply the same overflow checking as suggested above.
18787-18791: Add rank validation before accessing Shape indices.The method accesses
input.Shape[0],input.Shape[1], andweights.Shape[0]without validating that the tensors are 2D. If a 1D or 3D tensor is passed, this will throwIndexOutOfRangeExceptioninstead of a clear validation error. This is the same issue flagged in past review comments at lines 18607-18616.🔎 Suggested validation
+ if (input.Shape.Length != 2) + throw new ArgumentException($"Input must be 2D tensor, got {input.Shape.Length}D", nameof(input)); + if (weights.Shape.Length != 2) + throw new ArgumentException($"Weights must be 2D tensor, got {weights.Shape.Length}D", nameof(weights)); + // Input: [batch, inputFeatures], WeightsT: [outputFeatures, inputFeatures] // Result: [batch, outputFeatures] int m = input.Shape[0]; // batch size
18892-18896: Add rank validation before accessing Shape indices (double overload).Same missing rank validation as the float overload. Apply the same validation as suggested above for
inputandweightstensors.
🧹 Nitpick comments (1)
src/AiDotNet.Tensors/Engines/GpuEngine.cs (1)
1804-1807: Consider removing unused tiled kernel fields until ILGPU support is ready.The tiled kernel fields are disabled with
#pragma warning disable CS0169and marked as TODO. Since they're not used and the API is not finalized, consider removing them until the implementation is ready to avoid maintaining dead code.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/AiDotNet.Tensors/Engines/GpuEngine.cs
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: ooples
Repo: ooples/AiDotNet PR: 0
File: :0-0
Timestamp: 2025-11-19T04:08:26.895Z
Learning: For ILGPU GPU operations in GpuEngine.cs, use standard .NET exception types (InvalidOperationException, ArgumentException, OutOfMemoryException) instead of ILGPU-specific exception types, as ILGPU exception types may be version-specific. Combine with message-based filtering using ex.Message.Contains("device") or ex.Message.Contains("accelerator") as a fallback for GPU-specific errors.
📚 Learning: 2025-11-19T04:08:26.895Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 0
File: :0-0
Timestamp: 2025-11-19T04:08:26.895Z
Learning: For ILGPU GPU operations in GpuEngine.cs, use standard .NET exception types (InvalidOperationException, ArgumentException, OutOfMemoryException) instead of ILGPU-specific exception types, as ILGPU exception types may be version-specific. Combine with message-based filtering using ex.Message.Contains("device") or ex.Message.Contains("accelerator") as a fallback for GPU-specific errors.
Applied to files:
src/AiDotNet.Tensors/Engines/GpuEngine.cs
📚 Learning: 2025-12-18T08:49:25.295Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningMask.cs:1-102
Timestamp: 2025-12-18T08:49:25.295Z
Learning: In the AiDotNet repository, the project-level global using includes AiDotNet.Tensors.LinearAlgebra via AiDotNet.csproj. Therefore, Vector<T>, Matrix<T>, and Tensor<T> are available without per-file using directives. Do not flag missing using directives for these types in any C# files within this project. Apply this guideline broadly to all C# files (not just a single file) to avoid false positives. If a file uses a type from a different namespace not covered by the global using, flag as usual.
Applied to files:
src/AiDotNet.Tensors/Engines/GpuEngine.cs
📚 Learning: 2025-12-18T08:49:53.103Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningStrategy.cs:1-4
Timestamp: 2025-12-18T08:49:53.103Z
Learning: In this repository, global using directives are declared in AiDotNet.csproj for core namespaces (AiDotNet.Tensors.* and AiDotNet.*) and common system types. When reviewing C# files, assume these global usings are in effect; avoid adding duplicate using statements for these namespaces and for types like Vector<T>, Matrix<T>, Tensor<T>, etc. If a type is not found, verify the global usings or consider adding a file-scoped using if needed. Prefer relying on global usings to reduce boilerplate.
Applied to files:
src/AiDotNet.Tensors/Engines/GpuEngine.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL Analysis
- GitHub Check: Build (Windows)
🔇 Additional comments (3)
src/AiDotNet.Tensors/Engines/GpuEngine.cs (3)
150-321: LGTM: Well-designed tiled matrix multiply kernels.The tiled matrix multiplication implementation using shared memory is well-structured with proper synchronization, boundary checks, and comprehensive documentation explaining the 2-10x expected speedup for large matrices.
18415-18657: LGTM: Well-implemented adaptive matrix multiply with timing diagnostics.The implementation correctly:
- Uses
longfor overflow-safe operation counting (line 18416)- Applies TransposeThreshold=2048 to balance transpose cost vs cache miss benefit
- Adds comprehensive timing instrumentation for all phases (alloc, transfer, kernel, transfer back)
- Uses
AsSpan()andSubViewfor efficient memory operations- Properly cleans up conditional gpuBT buffer allocation
18659-18977: Excellent design for persistent GPU tensors (Phase B: US-GPU-030).The persistent GPU tensor allocation and cached-weights matrix multiply implementation effectively addresses the catastrophic transfer overhead. The design properly:
- Separates persistent allocation (AllocateGpuTensor) from pooled buffers
- Documents ownership model (caller must dispose)
- Uses optimized transposed-B kernel for coalesced memory access
- Includes comprehensive timing instrumentation for profiling
- Only transfers input data per operation, keeping weights GPU-resident
Minor issues flagged above regarding validation, but the core architecture is solid.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Implements cuBLAS-style tiled matrix multiplication using GPU shared memory for cache reuse. Key changes: - Load tiled kernels using LoadStreamKernel for explicit group config - Use 32x32 tiles (TileSize) for optimal shared memory utilization - Three-tier kernel selection based on matrix K dimension: - K >= 1024: Tiled kernel (shared memory, cuBLAS-style) - 512 <= K < 1024: Transpose-B kernel (coalesced access) - K < 512: Naive kernel (low overhead) - Both float and double precision supported - Proper grid/group configuration: grid=(M/32, N/32), group=(32, 32) Expected speedup: 2-10x for large matrices compared to naive kernel due to reduced global memory bandwidth from tile-based data reuse. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (8)
src/AiDotNet.Tensors/Engines/GpuEngine.cs (8)
2714-2714: Use structured logging instead of Console.WriteLine.Same as line 2607 - prefer structured logging over direct console output in library code.
297-302: Critical: Same indexing bug in double precision tiled kernel.The double precision kernel has the identical indexing error as the float version. Lines 298 and 300 must be corrected:
🔎 Proposed fix for tileBT loading (double)
// Load tile of B^T into shared memory - int bCol = t * TileSize + ty; + int bCol = t * TileSize + tx; if (col < p.N && bCol < p.K) - tileBT[tx, ty] = bT[col, bCol]; + tileBT[ty, tx] = bT[col, bCol]; else - tileBT[tx, ty] = 0.0; + tileBT[ty, tx] = 0.0;
18585-18597: Add 2D tensor rank validation (double precision version).Same validation issue as the float version - add rank checks before accessing
Shape[0]andShape[1].🔎 Add rank validation
private Tensor<double> MatMulGpuDouble(Tensor<double> a, Tensor<double> b) { + if (a.Shape.Length != 2) + throw new ArgumentException($"Tensor 'a' must be 2D, got {a.Shape.Length}D", nameof(a)); + if (b.Shape.Length != 2) + throw new ArgumentException($"Tensor 'b' must be 2D, got {b.Shape.Length}D", nameof(b)); + int m = a.Shape[0]; int k = a.Shape[1];
18790-18794: Potential integer overflow in shape validation (double precision).Same overflow risk as the float version at lines 18751-18755.
🔎 Use checked arithmetic or long type
- int expectedLength = 1; - foreach (int dim in shape) - expectedLength *= dim; - if (data.Length != expectedLength) - throw new ArgumentException($"Data length {data.Length} doesn't match shape {string.Join("x", shape)} = {expectedLength}"); + long expectedLength = 1; + foreach (int dim in shape) + expectedLength *= dim; + if (expectedLength > int.MaxValue) + throw new ArgumentException($"Shape dimensions too large: {string.Join("x", shape)} would overflow"); + if (data.Length != (int)expectedLength) + throw new ArgumentException($"Data length {data.Length} doesn't match shape {string.Join("x", shape)} = {expectedLength}");
18953-18964: Add rank validation before accessing Shape indices (double precision).Same rank validation issue as the float version at lines 18848-18859.
🔎 Add rank validation
if (input == null) throw new ArgumentNullException(nameof(input)); if (weights == null) throw new ArgumentNullException(nameof(weights)); if (gpuWeights == null) throw new ArgumentNullException(nameof(gpuWeights)); + + if (input.Shape.Length != 2) + throw new ArgumentException($"Input must be 2D tensor, got {input.Shape.Length}D", nameof(input)); + if (weights.Shape.Length != 2) + throw new ArgumentException($"Weights must be 2D tensor, got {weights.Shape.Length}D", nameof(weights)); + if (gpuWeights.Shape.Length != 2) + throw new ArgumentException($"GPU weights must be 2D tensor, got {gpuWeights.Shape.Length}D", nameof(gpuWeights)); // Input: [batch, inputFeatures], WeightsT: [outputFeatures, inputFeatures]
18751-18755: Potential integer overflow in shape validation.Multiplying dimensions without overflow protection could cause
expectedLengthto overflow for very large shapes, leading to incorrect validation.🔎 Use checked arithmetic or long type
- int expectedLength = 1; - foreach (int dim in shape) - expectedLength *= dim; - if (data.Length != expectedLength) - throw new ArgumentException($"Data length {data.Length} doesn't match shape {string.Join("x", shape)} = {expectedLength}"); + long expectedLength = 1; + foreach (int dim in shape) + expectedLength *= dim; + if (expectedLength > int.MaxValue) + throw new ArgumentException($"Shape dimensions too large: {string.Join("x", shape)} would overflow"); + if (data.Length != (int)expectedLength) + throw new ArgumentException($"Data length {data.Length} doesn't match shape {string.Join("x", shape)} = {expectedLength}");
18848-18859: Add rank validation before accessing Shape indices.The method accesses
input.Shape[0],input.Shape[1],weights.Shape[0], andgpuWeights.Shape[0],gpuWeights.Shape[1]without validating that tensors are 2D. Non-2D tensors will causeIndexOutOfRangeException.🔎 Add rank validation
if (input == null) throw new ArgumentNullException(nameof(input)); if (weights == null) throw new ArgumentNullException(nameof(weights)); if (gpuWeights == null) throw new ArgumentNullException(nameof(gpuWeights)); + + if (input.Shape.Length != 2) + throw new ArgumentException($"Input must be 2D tensor, got {input.Shape.Length}D", nameof(input)); + if (weights.Shape.Length != 2) + throw new ArgumentException($"Weights must be 2D tensor, got {weights.Shape.Length}D", nameof(weights)); + if (gpuWeights.Shape.Length != 2) + throw new ArgumentException($"GPU weights must be 2D tensor, got {gpuWeights.Shape.Length}D", nameof(gpuWeights)); // Input: [batch, inputFeatures], WeightsT: [outputFeatures, inputFeatures]
18447-18459: Add 2D tensor rank validation before accessing Shape indices.The code accesses
a.Shape[0],a.Shape[1],b.Shape[0], andb.Shape[1]without validating that the tensors are 2D. If 1D or 3D tensors are passed, this will throwIndexOutOfRangeExceptioninstead of a clear validation error.🔎 Add rank validation at method start
private Tensor<float> MatMulGpuFloat(Tensor<float> a, Tensor<float> b) { + if (a.Shape.Length != 2) + throw new ArgumentException($"Tensor 'a' must be 2D, got {a.Shape.Length}D", nameof(a)); + if (b.Shape.Length != 2) + throw new ArgumentException($"Tensor 'b' must be 2D, got {b.Shape.Length}D", nameof(b)); + int m = a.Shape[0]; int k = a.Shape[1];
🧹 Nitpick comments (1)
src/AiDotNet.Tensors/Engines/GpuEngine.cs (1)
2607-2607: Use structured logging instead of Console.WriteLine.Direct console writes in production library code can interfere with application output. Consider using a logging framework (e.g.,
ILogger) or removing this diagnostic output.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/AiDotNet.Tensors/Engines/GpuEngine.cs
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ooples
Repo: ooples/AiDotNet PR: 0
File: :0-0
Timestamp: 2025-11-19T04:08:26.895Z
Learning: For ILGPU GPU operations in GpuEngine.cs, use standard .NET exception types (InvalidOperationException, ArgumentException, OutOfMemoryException) instead of ILGPU-specific exception types, as ILGPU exception types may be version-specific. Combine with message-based filtering using ex.Message.Contains("device") or ex.Message.Contains("accelerator") as a fallback for GPU-specific errors.
📚 Learning: 2025-12-18T08:49:53.103Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningStrategy.cs:1-4
Timestamp: 2025-12-18T08:49:53.103Z
Learning: In this repository, global using directives are declared in AiDotNet.csproj for core namespaces (AiDotNet.Tensors.* and AiDotNet.*) and common system types. When reviewing C# files, assume these global usings are in effect; avoid adding duplicate using statements for these namespaces and for types like Vector<T>, Matrix<T>, Tensor<T>, etc. If a type is not found, verify the global usings or consider adding a file-scoped using if needed. Prefer relying on global usings to reduce boilerplate.
Applied to files:
src/AiDotNet.Tensors/Engines/GpuEngine.cs
📚 Learning: 2025-12-18T08:49:25.295Z
Learnt from: ooples
Repo: ooples/AiDotNet PR: 444
File: src/Interfaces/IPruningMask.cs:1-102
Timestamp: 2025-12-18T08:49:25.295Z
Learning: In the AiDotNet repository, the project-level global using includes AiDotNet.Tensors.LinearAlgebra via AiDotNet.csproj. Therefore, Vector<T>, Matrix<T>, and Tensor<T> are available without per-file using directives. Do not flag missing using directives for these types in any C# files within this project. Apply this guideline broadly to all C# files (not just a single file) to avoid false positives. If a file uses a type from a different namespace not covered by the global using, flag as usual.
Applied to files:
src/AiDotNet.Tensors/Engines/GpuEngine.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL Analysis
- GitHub Check: Build (Windows)
- Mark all Priority 1 layers as complete in checklist - Add progress log entry for Phase 3 continuation - Document skipped layers with reasons (incompatible types) - Update next steps for remaining phases 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add RegisterTrainableParameter and InvalidatePersistentTensor calls to enable GPU memory optimization across all trainable layers: Attention layers: - MultiHeadAttentionLayer, SelfAttentionLayer, CrossAttentionLayer - AttentionLayer, GraphAttentionLayer Recurrent/Memory layers: - RecurrentLayer, MemoryReadLayer, MemoryWriteLayer Embedding layers: - TimeEmbeddingLayer Convolutional layers: - Conv3DLayer, DeconvolutionalLayer, DeformableConvolutionalLayer - DepthwiseSeparableConvolutionalLayer, DilatedConvolutionalLayer - EdgeConditionalConvolutionalLayer, SeparableConvolutionalLayer - SubpixelConvolutionalLayer, ConvLSTMLayer Normalization layers: - BatchNormalizationLayer, LayerNormalizationLayer - GroupNormalizationLayer, InstanceNormalizationLayer Other layers: - EmbeddingLayer, PatchEmbeddingLayer, GRULayer, LSTMLayer 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move GpuDiagnostic.cs and GpuDiagnostic.csproj from root to tools/GpuDiagnostic/ folder. This fixes CI MSB1011 error where dotnet restore fails because multiple project files exist in root alongside AiDotNet.sln. Updated ProjectReference path to account for new location. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Math.Log2 and MathF.Log2 are only available in .NET Core 3.0+ and .NET 5.0+ respectively. This change uses the cross-framework compatible MathHelper.Log2 method instead. - Added float overload for MathHelper.Log2 - Updated CudaBackend, OpenClBackend, and CpuEngine FFT methods - Updated BucketPaddingStrategy for bucket size calculation - Updated Log2Operator SIMD methods 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…lity - Resolved merge conflicts in 8 files (InputHelper, ModelHelper, CapsuleLayer, GraphAttentionLayer, MessagePassingLayer, MixtureOfExpertsLayer, PositionalEncodingLayer, NonLinearRegressionBase) - Fixed PositionalEncodingLayer to use workingInput.Shape instead of input.Shape - Fixed HuggingFaceModelLoader for .NET Framework 4.7.1 compatibility: - Added using System.Net.Http for HttpClient - Replaced GetStringAsync with CancellationToken with single-arg version - Replaced ReadAsStreamAsync with CancellationToken with no-arg version - Replaced CopyToAsync with CancellationToken with no-arg version - Replaced File.Move with overwrite parameter with Delete+Move - Replaced ReadAsync/WriteAsync with Memory<byte> with 4-arg versions - Replaced SHA256.HashData with SHA256.Create().ComputeHash() - Replaced Convert.ToHexString with StringBuilder hex formatting - Fixed ONNXImporter for .NET Framework 4.7.1 compatibility: - Replaced BitConverter.Int32BitsToSingle with BitConverter.ToSingle 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Added IWeightLoadable<T> interface implementation to MockLayer<T>:
- GetParameterNames, TryGetParameter, SetParameter, GetParameterShape
- NamedParameterCount, ValidateWeights, LoadWeights
- Fixed CoreLayersIntegrationTests to use public SetParameter("weight", ...)
instead of protected SetWeights method
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ize, outputSize] The DenseLayer was using a non-standard weight matrix convention of [outputSize, inputSize], which required transposing weights during forward and backward passes. This conflicted with FusedLinear which expects the industry standard [inputSize, outputSize] convention. Changes: - Weight initialization now creates [inputSize, outputSize] tensors - Forward pass no longer requires weight transpose - Backward pass updated to use input^T @ gradient for weight gradients - Backward pass uses gradient @ weights^T for input gradients - EnsureWeightShapeForInput updated for new convention - ExportComputationGraph no longer transposes weights - BackwardViaAutodiff updated for new convention - Documentation updated to reflect new shape This fixes the "Weight matrix shape mismatch" errors in FusedLinear. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The NN-VLM-Other shard was consistently timing out at 45 minutes. Split into two shards: - 10a3a NN-VLM: BLIP, BLIP2, CLIP neural network tests - 10a3b NN-Adapters: LoRA, VBLoRA, AdvancedAlgebra, MoE tests Applied to both net8.0 and net471 test configurations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ntion - Remove duplicate ExtractFeaturesWithMeta method in GenreClassifier - Update LoRALayer.MergeWeights() to not transpose (A*B already produces [inputSize, outputSize]) - Fix all LoRA adapters to use correct indexing: i / outputSize, i % outputSize - Update DeltaLoRAAdapter matrix initialization to [inputSize, outputSize] - Update LoRALayerTests expected dimensions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ight convention - Update NeuralNetworkDerivatives.ProcessDenseLayer to use [inputSize, outputSize] convention - Fix weight indexing from weights[i, j] to weights[j, i] for correct gradient computation - Add TransposeWeights helper to QuantizedDenseLayer to convert weights for quantization - Update NeuralNetworkDerivativesTests parameters to match new weight convention 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The combined 10a3a NN-VLM shard was timing out at 45+ minutes. Split into three individual shards to target 15-20 minutes each: - 10a3a1 NN-Blip (BlipNeuralNetworkTests) - 10a3a2 NN-Blip2 (Blip2NeuralNetworkTests) - 10a3a3 NN-Clip (ClipNeuralNetworkTests) Applied to both net8.0 and net471 test matrices.
Enhanced the slow test reporting to identify code bottlenecks: - Show total test count, execution time, and average per test - List top 20 slowest tests with color-coded severity - Aggregate timing by test class (top 15) to identify problematic areas - Critical alerts for tests >60s (likely code bottlenecks) - Warning alerts for tests 30-60s (potential bottlenecks) This helps identify performance issues in the code under test.
Summary
Testing
Closes #496