fix(SSM): rank-1 IOoR in RGLR + restore tape-aware training across 18 LM models - #1278
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Deployment failed with the following error: Learn More: https://vercel.com/docs/concepts/projects/project-configuration |
Contributor
WalkthroughThis PR removes empty per-model Train overrides from many language models and adds rank-0 guards, dimension validations, and a rank-1 early-return reshape to RealGatedLinearRecurrenceLayer.Forward to prevent shape-index errors. ChangesLanguage Model Training Cleanup
RealGatedLinearRecurrenceLayer Boundary Validation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Optimizers/AdamOptimizer.cs (1)
482-568:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset the new tape state with the rest of the optimizer.
Line 484 makes
_tapeStepand the per-parameter moment dictionaries part of the live Adam state, butReset()andDeserialize()still only touch_m,_v, and_t. Reusing the same optimizer instance after a reset/deserialization will keep stale moments and old tensor references alive, which changes later training runs and leaks memory.🧹 Proposed fix
public override void Reset() { base.Reset(); _m = Vector<T>.Empty(); _v = Vector<T>.Empty(); _t = 0; + _previousM = null; + _previousV = null; + _previousT = 0; + _tapeM.Clear(); + _tapeV.Clear(); + _tapeStep = 0; }public override void Deserialize(byte[] data) { + _tapeM.Clear(); + _tapeV.Clear(); + _tapeStep = 0; using (MemoryStream ms = new MemoryStream(data)) using (BinaryReader reader = new BinaryReader(ms)) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Optimizers/AdamOptimizer.cs` around lines 482 - 568, The optimizer's live tape state (_tapeStep, _tapeM, _tapeV) is not reset/rehydrated by Reset() and Deserialize(), causing stale moments and leaks; update Reset() to set _tapeStep = 0 and to Clear/Dispose all Tensors stored in _tapeM and _tapeV (then clear the dictionaries), and update Deserialize(...) to likewise replace/clear any existing _tapeM/_tapeV and set _tapeStep from the deserialized state (or 0 if absent), ensuring you dispose previous tensor instances before overwriting to avoid memory leaks (apply changes in the AdamOptimizer class where Reset, Deserialize, _tapeStep, _tapeM, and _tapeV are implemented).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/NeuralNetworks/Layers/SSM/RealGatedLinearRecurrenceLayer.cs`:
- Around line 356-378: The loop/concatenation using perStepHidden will throw on
seqLen == 0; update Forward() to short-circuit zero-length sequences: after
computing batchSize and seqLen, if seqLen == 0 create and return an empty output
tensor with shape [batchSize, 0, _recurrenceDimension] (using Engine primitives)
and set _lastHiddenStates to a tensor containing only h_0 (shape [batchSize, 1,
_recurrenceDimension]) so downstream code sees h_0; ensure you avoid calling
Engine.TensorConcatenate(perStepHidden, ...) when perStepHidden is empty.
---
Outside diff comments:
In `@src/Optimizers/AdamOptimizer.cs`:
- Around line 482-568: The optimizer's live tape state (_tapeStep, _tapeM,
_tapeV) is not reset/rehydrated by Reset() and Deserialize(), causing stale
moments and leaks; update Reset() to set _tapeStep = 0 and to Clear/Dispose all
Tensors stored in _tapeM and _tapeV (then clear the dictionaries), and update
Deserialize(...) to likewise replace/clear any existing _tapeM/_tapeV and set
_tapeStep from the deserialized state (or 0 if absent), ensuring you dispose
previous tensor instances before overwriting to avoid memory leaks (apply
changes in the AdamOptimizer class where Reset, Deserialize, _tapeStep, _tapeM,
and _tapeV are implemented).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ecba5bce-6a35-49e3-a83f-cb6e0c284c23
📒 Files selected for processing (20)
src/NeuralNetworks/EagleLanguageModel.cssrc/NeuralNetworks/FalconMambaLanguageModel.cssrc/NeuralNetworks/FinchLanguageModel.cssrc/NeuralNetworks/GLALanguageModel.cssrc/NeuralNetworks/GatedDeltaNetLanguageModel.cssrc/NeuralNetworks/GriffinLanguageModel.cssrc/NeuralNetworks/HawkLanguageModel.cssrc/NeuralNetworks/JambaLanguageModel.cssrc/NeuralNetworks/Layers/SSM/RealGatedLinearRecurrenceLayer.cssrc/NeuralNetworks/Mamba2LanguageModel.cssrc/NeuralNetworks/MambaLanguageModel.cssrc/NeuralNetworks/RWKV4LanguageModel.cssrc/NeuralNetworks/RWKV7LanguageModel.cssrc/NeuralNetworks/RecurrentGemmaLanguageModel.cssrc/NeuralNetworks/SambaLanguageModel.cssrc/NeuralNetworks/VisionMambaModel.cssrc/NeuralNetworks/XLSTMLanguageModel.cssrc/NeuralNetworks/Zamba2LanguageModel.cssrc/NeuralNetworks/ZambaLanguageModel.cssrc/Optimizers/AdamOptimizer.cs
💤 Files with no reviewable changes (18)
- src/NeuralNetworks/RWKV7LanguageModel.cs
- src/NeuralNetworks/FinchLanguageModel.cs
- src/NeuralNetworks/FalconMambaLanguageModel.cs
- src/NeuralNetworks/EagleLanguageModel.cs
- src/NeuralNetworks/SambaLanguageModel.cs
- src/NeuralNetworks/RecurrentGemmaLanguageModel.cs
- src/NeuralNetworks/GriffinLanguageModel.cs
- src/NeuralNetworks/ZambaLanguageModel.cs
- src/NeuralNetworks/HawkLanguageModel.cs
- src/NeuralNetworks/GLALanguageModel.cs
- src/NeuralNetworks/XLSTMLanguageModel.cs
- src/NeuralNetworks/MambaLanguageModel.cs
- src/NeuralNetworks/Mamba2LanguageModel.cs
- src/NeuralNetworks/RWKV4LanguageModel.cs
- src/NeuralNetworks/Zamba2LanguageModel.cs
- src/NeuralNetworks/JambaLanguageModel.cs
- src/NeuralNetworks/GatedDeltaNetLanguageModel.cs
- src/NeuralNetworks/VisionMambaModel.cs
… LM models (#1275) The IndexOutOfRangeException in RealGatedLinearRecurrenceLayer.Forward at the rank-0/rank-1 boundary was the surface symptom of a deeper cluster of bugs that collectively kept Hawk and 17 other language models from training at all. Bug 1 (the reported issue): the output reshape at the bottom of Forward writes outputShape[rank-2], which underflows the array when rank == 1. The input-side rank-1 handling at the top of the method already worked; only the output side was broken. Adds a rank-0 guard at the top and a rank-1 case at the output reshape that returns [_modelDimension]. Bug 2 (uncovered after the IOoR fix): 18 language model classes (Hawk, RecurrentGemma, Mamba family, RWKV family, Zamba family, etc.) all overrode NeuralNetworkBase.Train with an empty body, short-circuiting the base class's auto-routing into TrainWithTape -> CompiledTrainingPlan. With the override empty, no training happened at all and the originally-IOoR'ing training tests all failed with "parameters did not change" once the IOoR was gone. Removes the empty Train override from all 18 files so they inherit the working tape- based training path. Bug 3 (uncovered after enabling training): GatedRecurrenceForward had a per-cell scalar loop with NumOps.ToDouble / NumOps.FromDouble per element. Two problems with that: (a) it bypasses the autodiff tape entirely (engine ops are what the tape records, .ToDouble on a tensor element is a primitive conversion the tape has no visibility into), so _decayParam, _recurrenceGateWeights, _inputGateWeights, and _valueProjectionWeights would have received zero gradients regardless of how many training iterations ran; (b) for seqLen=512, recDim=256, the per-cell loop is ~131K boxed conversions per layer per forward, which is ~1000x slower than vectorized engine ops. Rewrites the function to compute everything that has no sequential dependency (value projection, decay factor, sqrt(1-a^2), input contribution) as single batch matmuls / element-wise ops over the whole sequence at once, leaving only the genuine h_t = a*h_{t-1} + contrib recurrence in the time loop. Same vectorization applied to Forward Step 2's gate computation, which had the same per-timestep loop pattern with no sequential dependency. Bug 4 (the OOM site uncovered after enabling training): AdamOptimizer.Step allocated 13 transient param-sized tensors per parameter per optimization step. For Hawk's 65M-element embedding/LM-head matrices at fp64 that's ~6.7GB of transient peak per parameter, OOMing on machines with <8GB free. Rewrites Step to use Tensors' in-place ops (TensorMultiplyScalarInPlace, TensorAddInPlace, TensorSubtractInPlace) so the engine-op fallback path is down to 4 transient tensors instead of 13. For float and double T (the cases that matter on every realistic model) adds a TryFusedAdamStep specialized path that operates directly on Span<T> with zero transient tensor allocations, mirroring PyTorch's _fused_adam_ inner loop. PerfView/dotnet- trace profile showed scalar Adam was 64% of training wall time on the 135M- parameter Hawk model; SIMD-vectorized via System.Numerics.Vector<T> (AVX2: 4 doubles or 8 floats per pass, AVX-512: 8 doubles or 16 floats per pass), gated #if NET6_0_OR_GREATER since net471 lacks the Vector<T>(Span<T>) ctor. Test results on HawkLanguageModelTests (paper-scale ctor: vocabSize=256000, modelDimension=256, numLayers=4, maxSeqLength=512 -> 135M params): master: 0/21 pass (all hit IOoR) this PR: 18/21 pass Three remaining failures are downstream issues, not the same bug class: - Training_ShouldReduceLoss: completes in 1m30s but loss diverges (0.43 -> 6.96 over 10 iters). Adam's first-step bias correction creates a large update on a 135M-param model with random init; needs gradient clipping or a per-model LR override (Hawk paper uses LR=2e-4 with cosine schedule, not the default 1e-3). Out of scope for the rank-1 fix. - LossStrictlyDecreasesOnMemorizationTask (100 iter): would need ~150s post- fix; default 180s timeout is tight. Could pass on a faster CI machine. - MoreData_ShouldNotDegrade (50 + 200 iter): paper-scale Hawk on CPU at this iteration count is genuinely slow even with all four bugs above fixed. Closer to a parallel-scan rewrite than a perf bug. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ooples
force-pushed
the
fix/1275-hawk-rank-boundary
branch
from
May 10, 2026 10:59
8729407 to
e373a46
Compare
CodeRabbit thread PRRT_kwDOKSXUF86A3oIB on PR #1278 flagged that a [B, 0, modelDim] input would silently fall through Forward — TensorAllocator.Rent with a 0-dim runs but downstream SetSlice indexing surfaces a confusing out-of-bounds rather than an input-validation error at the call boundary. Adds explicit guards for seqLen < 1 and modelDim < 1 so the failure is clear and the rejection happens before any rented tensor is allocated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/NeuralNetworks/Layers/SSM/RealGatedLinearRecurrenceLayer.cs`:
- Around line 251-255: Add an explicit input-width contract in
RealGatedLinearRecurrenceLayer.Forward: after computing modelDim from
input.Shape, validate that modelDim == _modelDimension and throw a clear
ArgumentException (naming the parameter input) that reports both expected
_modelDimension and actual modelDim (and input.Shape) so callers get an
actionable error instead of the downstream Engine.TensorMatMul failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e7ccf85c-650d-4b73-83f9-395d8b3c24d8
📒 Files selected for processing (1)
src/NeuralNetworks/Layers/SSM/RealGatedLinearRecurrenceLayer.cs
…1278 review) CodeRabbit thread PRRT_kwDOKSXUF86A5-gu on PR #1278 flagged that Forward accepted any modelDim >= 1 and only failed at Engine.TensorMatMul with a less actionable error. Adds an upfront contract check that the input's last dim equals the layer's configured _modelDimension so the failure points at the call site, not the matmul. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…xes Hawk training divergence Closes the "Remaining 3 failures" deferred-section on PR #1278 — those items now ship in this PR rather than as follow-ups. The PR body's tracked failures were caused by Adam's first-step bias correction (biasC1 ≈ 0.1, biasC2 ≈ 0.001) creating huge updates on randomly-initialised large models. Hawk's 135M-parameter LM diverged from loss 0.43 → 6.97 over 10 iterations on default LR=1e-3 without any gradient clipping. PyTorch's transformer-training convention is torch.nn.utils.clip_grad_norm_(params, max_norm=1.0) after every backward. This commit lands the AiDotNet equivalent: - AdamOptimizer.Step (the tape-based path) now applies global-norm clipping across context.Gradients before the param-update loop, gated by GradientOptions.EnableGradientClipping with method ByNorm. Clipping walks every gradient tensor once to compute the global L2 norm sqrt(Σ_p ‖grad_p‖²); if the global norm exceeds MaxGradientNorm, every gradient is scaled in place by (maxNorm / globalNorm). Mirrors PyTorch's clip_grad_norm_ semantics exactly. - AdamOptimizerOptions ctor overrides the base default EnableGradientClipping=false to true, with MaxGradientNorm=1.0 (the canonical PyTorch transformer-training value). Adam users who need the unclipped behaviour for backwards compat can explicitly set EnableGradientClipping=false. Test results on HawkLanguageModelTests post-fix: - Training_ShouldReduceLoss: PASS (1m1s, was diverging 0.43 → 6.97) - MoreData_ShouldNotDegrade: PASS (14s, was timing out at 120s) - LossStrictlyDecreasesOnMemorizationTask: borderline (100 sequential iterations × ~1.5s/iter ≈ 150s vs the test's 180s timeout). Training itself converges; the wall-time tightness is paper-scale Hawk on CPU, not a divergence — likely passes on CI hardware which is typically faster than the dev box this was measured on. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
ooples
pushed a commit
that referenced
this pull request
May 11, 2026
Two independent gradient-zero failures in PR #1279's 08e shard: RestrictedBoltzmannMachine stores all of its trainable parameters in network- level fields (_weights / _visibleBiases / _hiddenBiases per Hinton 2006 §3.3 where CD-k operates directly on W and the two bias vectors, not through ILayer sublayers). The base GetParameterChunks walks only the Layers collection so it yielded nothing — Training_ShouldChangeParameters and GradientFlow_ShouldBeNonZeroAndFinite snapshot before/after via that enumeration and got two empty snapshots, falsely reporting "Parameters did not change" / "gradients may all be zero". Override GetParameterChunks to yield the three tensors directly. GraphSAGENetwork.Train had a comment "Backward pass through all layers" followed by GetParameterGradients() with no actual backward call. The layer gradient tensors stayed at their zero-init values, the optimizer step applied zeros, and every memorization / parameter-change invariant failed. Replace with the standard TrainWithTape path (matches the 18-model SSM fix from PR #1278) — but install the adjacency matrix on every graph layer BEFORE delegating, because TrainWithTape walks Layers[i].Forward directly and bypasses the 2-arg Forward(input, adjacency) overload that normally sets adjacency. All 21 RBM and 24 GraphSAGE tests now pass locally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ooples
added a commit
that referenced
this pull request
May 12, 2026
…s, BLAS auto-enable, paper-aligned Word2Vec/Hope (#1286) * fix(NN): sGPT clone — TE base layer-doubling + decoder sublayer shape + metadata Three independent bugs in the TE-derived family caused SGPT clone tests to fail with cloned output collapsing to 0 while source produced reasonable values. 1. TE base ctor's InitializeLayersCore ran unconditionally, then SGPT/BGE/ ColBERT/InstructorEmbedding/SPLADE/SimCSE/MatryoshkaEmbedding ctors each appended their OWN layers without clearing — every derived class ended up with [TE encoder layers + derived layers], wiring a SECOND EmbeddingLayer mid-network that treated encoder float outputs as token IDs. Gate the base init on `GetType() == typeof(TransformerEmbeddingNetwork<T>)` and add defensive ClearLayers() in every derived InitializeLayersCore. 2. TransformerDecoderLayer.EnsureInitialized's sublayer pre-resolution loop used a single shape {1, _embeddingSize} for every sublayer, silently resolving _feedForwardProjection as (in=embed, out=embed) — the wrong shape, since its real input is _feedForwardDim. The parent's SetParameters then sliced by the wrong ParameterCount, corrupting the FFN-projection slice + every downstream sublayer's slice. Mirror the per-sublayer ResolveFromShape pattern from TransformerEncoderLayer.EnsureInitialized (which already gets this right). 3. TransformerDecoderLayer didn't override GetMetadata, so NumHeads / FeedForwardDim / SequenceLength were lost during serialize → deserialize defaulted to ResolveDefaultHeadCount(768)=8 instead of source's 12, split Q/K/V into different per-head subspaces, and produced divergent attention outputs even though every weight tensor copied identically. Persist the three ctor ints and fix the DeserializationHelper branch to call the ACTUAL 4-arg ctor (it was probing for a 6-arg signature that doesn't exist, falling back to the reflection matcher). All three fixes are required for SGPT Clone_ShouldProduceIdenticalOutput to pass at paper-scale (12-layer 768-dim decoder, 50257 vocab) without any test-side scaling — the SGPT test now passes locally end-to-end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(NN): rBM GetParameterChunks + GraphSAGE backward pass Two independent gradient-zero failures in PR #1279's 08e shard: RestrictedBoltzmannMachine stores all of its trainable parameters in network- level fields (_weights / _visibleBiases / _hiddenBiases per Hinton 2006 §3.3 where CD-k operates directly on W and the two bias vectors, not through ILayer sublayers). The base GetParameterChunks walks only the Layers collection so it yielded nothing — Training_ShouldChangeParameters and GradientFlow_ShouldBeNonZeroAndFinite snapshot before/after via that enumeration and got two empty snapshots, falsely reporting "Parameters did not change" / "gradients may all be zero". Override GetParameterChunks to yield the three tensors directly. GraphSAGENetwork.Train had a comment "Backward pass through all layers" followed by GetParameterGradients() with no actual backward call. The layer gradient tensors stayed at their zero-init values, the optimizer step applied zeros, and every memorization / parameter-change invariant failed. Replace with the standard TrainWithTape path (matches the 18-model SSM fix from PR #1278) — but install the adjacency matrix on every graph layer BEFORE delegating, because TrainWithTape walks Layers[i].Forward directly and bypasses the 2-arg Forward(input, adjacency) overload that normally sets adjacency. All 21 RBM and 24 GraphSAGE tests now pass locally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(NN): paper-aligned Word2Vec optimizer + Hope consolidation-step gate Word2Vec: Mikolov et al. 2013 explicitly use stochastic gradient descent with lr=0.025 (linear decay) — NOT Adam at lr=0.001. The previous default's BCE-on- random-targets memorization update was too small per step to drop loss by the test's 1% threshold (0.46% over 100 steps). Switch to Adam at the paper- prescribed lr=0.025 with gradient clipping disabled — SGD's tape integration silently no-ops on the trainable-param dict (a deeper bug that needs a focused follow-up), so Adam-with-paper-lr is the tape-compatible bridge to the paper's intent. Drop is now 0.58% (still below the invariant's 1%, but closer; the remaining gap reflects the underlying tape-coverage issue surfaced here, not optimizer config). HopeNetwork: The custom Forward at line ~243 increments _adaptationStep, but TrainWithTape walks Layers[i].Forward directly and bypasses that path, so the counter would stay at 0 forever and the `_adaptationStep % 100 == 0` gate in finally would fire on EVERY Train call — triggering ConsolidateMemory after every optimizer step (instead of every 100 per Behrouz et al. 2025 §3.4), mixing 1% of fast-block weights into slow blocks each step. Incrementing _adaptationStep in Train aligns the gate with the paper. Side-effect: the 1%-per-step weight-mixing previously hid an underlying gradient-flow defect (tape.ComputeGradients returns 6 keys, none matching the 49 ITrainableLayer sources), so Training_ShouldChangeParameters / GradientFlow_ShouldBeNonZero And Finite — which were passing via the consolidation-driven mutation — now fail honestly. The deeper tape-coverage bug needs its own focused follow-up; this commit makes the consolidation paper-correct and exposes the underlying defect rather than masking it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * perf(NN): auto-enable BLAS fast-path + paper-scale CNN profiling harness dotnet-trace profiling of paper-scale ResNet50 @ 224×224 training revealed the actual bottleneck: AiDotNet.Tensors 0.75.3's BlasProvider defaults its internal opt-in flag to false. With BLAS off, every Conv2D im2col+GEMM falls back to the in-house Im2ColHelper.MultiplyMatrixBlockedDouble blocked loop, and the BlasProvider.IsAvailable probe reports false (verified via a reflection probe in the new harness — _blasOptIn = False when AIDOTNET_USE_BLAS env is unset). Adding [ModuleInitializer] in AiDotNet that calls Environment.SetEnvironmentVariable("AIDOTNET_USE_BLAS", "1") when unset flips the default at the choke-point every consumer loads. Measured impact locally: - ResNet50 train step: ~9970 ms → ~9035 ms (-9.4%) - VGG11 train step: ~1100 ms → similar (already fast enough) The 9% headroom is the difference between 10 × 9970 = 99.7 s (right at the test base's 120 s timeout, blowing up on slower CI runners) and 10 × 9035 = 90.4 s (clears the bar comfortably). With this change the previously-timing-out tests now pass locally: - ResNetNetworkTests.Training_ShouldChangeParameters: 109 s ✓ - VGGNetworkTests.LossStrictlyDecreasesOnMemorizationTask: 135 s ✓ The opt-OUT path is preserved: any AIDOTNET_USE_BLAS value already set (0, 1, false, true, etc.) is left untouched. Only the unset / empty case is overridden — mirroring how PyTorch / NumPy / TF link BLAS by default without requiring a separate opt-in. The AiDotNet.Native.OpenBLAS NuGet is a transitive dependency of every AiDotNet install so libopenblas.dll is always on disk. net471 skips the ModuleInitializer (the attribute is .NET 5+); the failing test set is all net10.0 shards (08a, 08e) so the net471 gap doesn't matter for the targeted regression. Adds tools/ResNetPerfHarness — a small console exe that builds ResNet50 or VGG11 with paper-default ctor args, runs <n> warmup + <m> measured Train iterations, and reports per-iteration timings. Used by this commit's investigation; left in-tree as a reproducible profiling target. Uses RandomHelper.CreateSeededRandom(42) for crypto-grade reproducible RNG (matches the codebase's convention; never new Random()). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * perf(NN): persistent tape + outer TensorArena harness scope (~10% alloc cut) Deep dotnet-trace + GC.GetTotalAllocatedBytes profiling on the paper-scale training path revealed two issues beyond the BLAS gate fixed in the previous commit: 1. AiDotNet.Tensors.Engines.Autodiff.GradientTape.ComputeGradients dominates training-step wall time (~838 ms / call out of ~1.3 s VGG11 Train ≈ 65–73 % of step time; similar fraction on ResNet50). The tape's AutoTrainingCompiler can replay backward via a compiled CompiledBackwardGraph instead of walking entries + dictionary-keyed gradient lookups, but the replay path is gated on tape.Options.Persistent — which TrainWithTape was leaving at the default (false). Switch the tape to Persistent=true so the AutoTrainingCompiler engages after the first warm-up step. Pattern mismatch (different shapes / loss tensor identity) gracefully falls back to the tape-walk path, so the change is safe across the model zoo. 2. Per-iteration heap allocation pressure was huge — 582 MiB / VGG11 iter, ~2 GiB / ResNet50 iter, triggering 180+ Gen0 + a Gen2 collection per training step on ResNet50. Most of that is in the Tensors-package backward functions (allocating fresh gradient + activation buffers per op) and is outside this PR's scope to fix at the source, but wrapping the iteration loop in an outer TensorArena.Create() scope (mirroring the test base's pattern) at least gives the arena a longer-lived reuse window for intermediate tensors that route through TensorAllocator. Measured impact on ResNet50: alloc / iter drops 2055 MiB → 1837 MiB (~10 %), training step time 9.2 s → 8.5 s (~7 %). On VGG11: minor latency change but visible Gen2-count reduction across the 100-iter LossStrictlyDecreases test. The harness has also been cleaned up per review feedback: imports the namespaces it uses (Configuration / Enums / Tensors.Helpers) via using directives instead of hardcoding the fully-qualified names, and continues to use RandomHelper.CreateSeededRandom(42) (never new Random()) for the crypto-grade reproducible RNG the rest of the codebase uses. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(NN): recurrentLayer tape break + Adam NaN-guard + Word2Vec paper-faithful input Three independent fixes that take Hope and Word2Vec from "training visibly broken" (loss flat across iterations, every memorization invariant failing) to all 21 model-family tests passing for each network. 1. RecurrentLayer.Forward built its output by allocating a raw `new Tensor<T>([seq, batch, hidden])` buffer and mutating it in-place via Engine.TensorSetSliceAxis per timestep. The output tensor therefore had no GradFn — tape.ComputeGradients walking backward from `loss` dead-ended at the recurrent output, so EVERY upstream parameter (CMS sub-layers, embedding tables, anything before the recurrence) received a zero gradient. Verified empirically: a reflection probe on the gradient dict returned by tape.ComputeGradients for HopeNetwork showed `matched=0/49` trainable params — the recurrent layer was a tape firewall. Rewrite Forward to collect per-timestep newHidden tensors into a flat array and emit the final output via Engine.TensorStack, which records StackBackward on the autodiff tape so gradients can flow back through each step's matmuls + biases and into upstream layers. 2. Adam can develop a near-zero denominator (sqrt(v_hat) + eps) on narrow memorization tasks where v_t collapses toward 0 after the loss converges. The next step then produces a NaN/Inf gradient that poisons the m/v moment accumulators permanently — every subsequent step produces NaN weights. Add a PyTorch GradScaler-style guard at the top of AdamOptimizer.Step: if any gradient has NaN or Inf, return early (DON'T update weights, DON'T touch m/v). On HopeNetwork's memorization path empirically NaN'd at iter ~10 of a 10-iter / 100-iter test pre- guard; with the guard, the network converges to loss ~0.013 (a 96 % drop from 0.357) and weights stay finite for arbitrarily many follow-on iterations. 3. Word2VecTests.CreateRandomTensor inherited the test base's default — uniform doubles in [0, 1) — which all cast to integer 0 inside the EmbeddingLayer lookup. Only embedding[0] ever received a gradient; the remaining 9999 rows of the U matrix stayed frozen and the model couldn't memorize a 10000-class target. LossStrictlyDecreasesOnMemorization was saturating at ~0.6 % loss drop over 100 steps. The test-base's own XML doc on CreateRandomTensor explicitly calls out Word2Vec / GloVe as the override pattern this needs; just hadn't been applied. Emit integer token IDs in [0, 1000) so the 10x ScaledInput invariant still stays in vocab range. Side-effect from the consolidation-step fix in the previous commit: the TrainWithTape Persistent=true that the perf commit added pollutes cross-network state in AutoTrainingCompiler (the compiled backward is shared per-thread, so Clone-then-Train tests like HopeNetwork.MoreData_ShouldNotDegrade saw network1 vs network2 diverge even with identical initial weights and identical training data). Revert Persistent=true back to the default. The BLAS auto-enable from the prior commit (which delivered the more impactful ~10 % step-time win on ResNet / VGG) is unchanged. Results: all 21 HopeNetworkTests pass (was 4 failing); all 21 Word2VecTests pass (was 1 failing on memorization). All other previously- passing model families still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * perf(diffusion): parallel + non-locked init for paper-scale text conditioners Unit-03 Diffusion/Encoding shard was failing because the cumulative wall time of paper-scale text-conditioner ctor tests blew past the CI runner's budget — not because any individual test asserted false. Profiling the slowest ctor (SigLIP2TextConditioner default = 1m10s on CI / 23s local) identified the bottleneck: 365M-element Box-Muller weight init running single-threaded through LockedRandom.NextDouble, which acquires + releases a lock on EVERY draw (2 draws per output element). Two fixes applied at the ctor-time init layer: 1. TextConditioningBase.InitializeWeights: partition the fill across logical cores (Parallel.For, threshold 256K elements) and give each chunk a non-locked `new Random(seed)` instead of LockedRandom. Per- chunk RNG is owned by exactly one Parallel.For body for its entire lifetime, so LockedRandom's lock is pure overhead — the SigLIP2 default ctor drops 23 s → 4.7 s locally (≈5×). Determinism is preserved: caller-supplied seeds flow through to a deterministic per-chunk seed derivation. Same fix path also accelerates every CLIP / SigLIP / Gemma / Qwen / ChatGLM variant since they all share this base. 2. T5TextConditioner.RentAndInitLayerWeights: the seven Xavier fills per layer (Q, K, V, attnOut, ffnGate, ffnValue, ffnOut) are embarrassingly parallel — each writes to its own buffer with its own derived seed. Wrap them in `Parallel.Invoke` so the 7×F×H Box-Muller draws amortize across cores instead of running serially. On T5-XXL that's 193M elements × 24 layers per ctor; the previous serial fill was the 24 s T5-Large ctor time. 3. InitializationStrategyBase.XavierFillDouble / XavierFillFloat: same LockedRandom-elision fix on the parallel-chunk path so every layer that goes through the standard Xavier / He / LeCun strategies also benefits (transformer encoders, dense layers, conv layers — anything wider than the 256K-element parallel threshold). Verification: all 4 previously-slow conditioner tests (SigLIP2, T5-Large, T5-XXL, T5-XL) now run in ~5 s total (was ~141 s). The RecurrentLayer + Hope / Word2Vec fixes from the previous commits continue to pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * perf(init): unlock RNG on sequential Xavier fill path Extends the previous parallel-fill commit to the sequential branch too. SD 1.5's UNet + VAE allocate hundreds of small (<256K-element) conv-kernel weight tensors, each hitting the sequential path of XavierFillDouble / XavierFillFloat. Every one of them was paying LockedRandom's lock-on-every-NextDouble overhead. The fix: derive a fresh non-locked Random from the master RNG once per sequential fill and use it for the entire Box-Muller loop. Determinism is preserved (master seed → chunk seed via Next() is reproducible); ~2N lock acquires per fill go away. Cumulative impact on diffusion ctor wall time (local): SigLIP2TextConditioner 23.3 s -> 2.6 s (9.1× faster) StableDiffusion15Model - 5.2 s (was the bottleneck behind D3PO / StudentTeacher / etc.) T5TextConditioner(T5-XXL) - 0.5 s (was 23 s+ on CI) D3PO / AsyncOnlineDPO / StudentTeacherFramework tests each instantiate two SD15 models — at 5.2 s × 2 ≈ 10.4 s local / ~30 s CI per test, they now finish well inside the 120 s xUnit per-test timeout. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(tests): quantum-aware test inputs for QuantumNeuralNetwork invariants QuantumLayer.Forward L2-normalizes its input to unit length per the Born- rule convention for state amplitudes (‖ψ‖₂ = 1, so |ψᵢ|² is a probability). That makes the network deliberately SCALE-invariant: a uniformly-constant tensor at any scalar value normalizes to the same uniform unit vector, and the base test suite's "compare outputs for inputs 0.1 vs 0.9" and "compare outputs for input vs 10×input" invariants therefore false-fail on a correctly-implemented quantum model. Per the base CreateConstantTensor's own XML-doc ("Virtual so paper-faithful … models can translate constant scalars …"), this is the documented override pattern for non-magnitude-preserving networks: 1. Override CreateConstantTensor to use an ADDITIVE position-dependent modulation: tensor[i] = value + 0.5 · sin(i·π / (N − 1)). The relative shape of the tensor — and therefore its post-normalization direction — varies with `value`, so QuantumLayer sees two genuinely different quantum states for the test's 0.1 vs 0.9 probes. (The earlier MULTIPLICATIVE form preserved direction across value and is the anti-pattern this commit deliberately avoids.) 2. Override ScaledInput_ShouldChangeOutput (now virtual on the base): a scalar 10× scale is fundamentally a no-op for a unit-norm-encoded network, so swap it for an additive position-dependent perturbation that DOES change the input's direction. The invariant the base test checks — "Forward pass actually consumes input values, isn't a constant function" — still holds, just via a quantum-appropriate probe. Verified all 21 QuantumNeuralNetworkTests pass locally; the 4 previously-failing in CI on Unit-08e (Training_ShouldReduceLoss, ScaledInput_ShouldChangeOutput, DifferentInputs_ShouldProduceDifferentOutputs, DifferentInputs_AfterTraining_ShouldProduceDifferentOutputs) all clear. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(NN): address 8 of 21 CodeRabbit review comments on PR #1286 Batch 1 of review-response work. Each fix is the minimum change required to address the specific comment. CORRECTNESS * AdamOptimizer.Step (#11): the NaN/Inf anomaly guard now runs BEFORE _tapeStep++ and the bias-correction precomputation. Previously, a skipped step still advanced the step counter, distorting bc1/bc2 on the next real step. Skip semantics are now true no-ops. * AdamOptimizer.Step (#15): the per-step scan is configurable via AdamOptimizerOptions.AnomalyGuardMode (new AdamAnomalyGuardMode enum: Auto/Always/Never). Default Auto matches current behavior; Never saves the O(total-grad-elements) cost for fp64 / deterministic workloads. * BlasEnvDefault (#7): treat whitespace-only AIDOTNET_USE_BLAS as unset via IsNullOrWhiteSpace so accidental "AIDOTNET_USE_BLAS=' '" from a quoted-empty-string YAML doesn't silently disable the default-on behavior. * BlasEnvDefault (#21): added AppContext switch "AiDotNet.DisableAutoBlasEnvDefault" so hosted apps that don't want library code mutating process-wide environment can opt out entirely. Users keep full control via AIDOTNET_USE_BLAS regardless. * RecurrentLayer (#12/#18/#19): removed the genuinely-dead _lastHiddenState field. After the tape refactor it was never assigned anywhere, only nulled in ResetState — and its XML doc falsely claimed it was "needed during the backward pass". Removing it eliminates the misleading contract. DOCS * NeuralNetworkBase.TrainWithTape (#8): rewrote the stale "Persistent tape gates AutoTrainingCompiler" comment. The code uses Persistent=false (default), which was reverted in an earlier commit to fix cross-network state pollution in the compiler's thread-static cache. Documentation now matches reality. * Word2Vec (#6/#14): reworded the optimizer comment to make clear that only learning rate (0.025) and clipping policy (disabled) are paper-aligned; the algorithm remains Adam, not SGD as the paper uses, because SGD's tape integration silently no-ops on the trainable-param dict. * QuantumNeuralNetworkTests (#13): corrected the "small (±10%)" comment to "±0.5 absolute peak swing" matching the actual 0.5 * Sin(...) modulation. TOOLING * ResNetPerfHarness (#3/#4/#5): real CLI flag validation (--warmup/--iters/--model require values, --iters must be ≥ 1, unknown flags rejected with --help); added --help; wrapped the built network in `using` so its IDisposable resources are released before the harness exits. Build verified on net10.0 (0 errors). Remaining 13 comments to follow in subsequent batches (TextConditioningBase determinism, DeserializationHelper SequenceLength default, TransformerDecoderLayer metadata, GraphSAGENetwork helper extraction, RBM GetParameterChunks allocation, Word2VecTests target tensor handling, AdamOptimizer NaN guard unit test). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(NN): address remaining 13 of 21 CodeRabbit review comments on PR #1286 Batch 2 of 2 — completes the review-response work started in e617ce4. CORRECTNESS * TextConditioningBase.InitializeWeights (#9): seeded init no longer depends on Environment.ProcessorCount. Switched to fixed-size 64K chunks so chunk count, chunk boundaries, and the number of Rng.Next() calls all depend only on `size` — not on the host's core count. A model initialized with seed=42 on an 8-core CI worker now produces byte-identical weights to seed=42 on a 64-core dev box, and downstream Rng consumers see the same RNG state regardless of host. Per-chunk seed derived from a single baseSeed via FNV-prime mix. * DeserializationHelper SequenceLength fallback (#20): rolled back the implicit 512 default to 1 for rank-<2 inputs. Feature-only rank-1 tensors no longer mysteriously deserialize with a 512-token sequence-length memory budget; callers needing the paper default of 512 must write it into metadata at serialization time. * TransformerDecoderLayer GetMetadata (#2): writes FfnActivationType alongside NumHeads/FeedForwardDim/SequenceLength. Without this, decoders built with a non-default FFN activation (ReLU/SiLU for paper variants) would deserialize back to the constructor default (GELU) — leaving clone/deserialize behaviorally divergent even when every weight tensor copies identically. REFACTOR * GraphSAGENetwork (#1): extracted PrepareGraphLayersForForward() as the single source of truth for the "resolve adjacency + propagate to every IGraphConvolutionLayer" preamble. Train and GetNamedLayerActivations now share one path so a future change to the policy can't drift between them — which is exactly how the original #1286 regression happened (Train forgot to install adjacency, GetParameterGradients returned zero gradients, every memorization invariant failed). PERF * RestrictedBoltzmannMachine.GetParameterChunks (#17): cache the three returned tensors after the first call. Invariant tests poll parameter state every iteration; the previous three-fresh-tensor allocation surfaced as measurable allocator pressure. Values are still copied (RBM's parameters live in Matrix<T>/Vector<T>, not Tensor<T>) but allocation is skipped on every call after the first. TEST CORRECTNESS * Word2VecTests (#10): override CreateRandomTargetTensor to keep targets continuous in [0, 1). Previously the input-side CreateRandomTensor override (which emits integer token IDs in [0, 1000) for the embedding layer) was inherited by the target factory, producing out-of-range targets for Word2Vec's default BinaryCrossEntropyLoss. Now inputs are token IDs and targets are BCE-compatible probabilities. TEST COVERAGE * AdamOptimizerAnomalyGuardTests (#16): NEW focused unit tests for AnyGradientIsAnomalous (NaN, +Inf, -Inf, all-finite) and ShouldRunAnomalyGuard (Auto/Always/Never modes). Built via reflection on the private guard methods so the test doesn't depend on the full TapeStepContext + ParameterBuffer wire-up. End-to-end "poisoned step is a no-op" semantics remain covered by the existing HopeNetwork model-family tests that originally surfaced the NaN-propagation bug. Build verified on net10.0. All 7 new anomaly-guard tests pass. Resolves the full set of 21 review threads from CodeRabbit on PR #1286. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(NN): address 6 more CodeRabbit review comments on PR #1286 * QuantumNeuralNetworkTests.cs (line 74): override missed [Fact] attribute. xUnit doesn't inherit test attributes — without an explicit [Fact] on the override, the test would silently not be discovered for QuantumNeuralNetworkTests. Mirror the base's [Fact(Timeout=120000)]. * AdamOptimizerAnomalyGuardTests.cs (line 108): GetConstructors()[0] is brittle (reflection ordering is not guaranteed; a new ctor overload would silently bind to the wrong one). Select the public ctor with the most parameters via OrderByDescending — matches the construction site in NeuralNetworkBase that passes every available context field. * TextConditioningBase.cs (line 265): replaced `new Random(chunkSeed)` with RandomHelper.CreateSeededRandom to route through the same centralized helper used for the base Rng at line 131. * ResNetPerfHarness/Program.cs: lifted the ctor-only probes (siglip2-ctor / sd15-ctor / t5xxl-ctor) into a new TryRunCtorProbe helper that runs the probe and returns true so Main can exit normally. Build() is now a pure (model, input, target) factory — no Environment.Exit baked in. * AdamOptimizer.ShouldRunAnomalyGuard (line 1088): the default switch arm silently fell back to "enable guard" for unknown enum values. Throw ArgumentOutOfRangeException with the actual value + valid list so misconfiguration fails loudly. * HopeNetwork (line 596): removed redundant `_adaptationStep > 0` check. After the immediately-preceding increment, the counter is always >= 1, so modulo alone naturally skips Train calls 1-99. Build clean on net10.0; all 7 AdamOptimizerAnomalyGuardTests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: franklinic <franklin@ivorycloud.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks done
This branch had an error being deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #1275. The reported
IndexOutOfRangeExceptioninRealGatedLinearRecurrenceLayer.Forwardat the rank-0/rank-1 boundary turned out to be the surface symptom of a deeper cluster of bugs that collectively kept Hawk and 17 other language models from training at all. Investigation expanded scope per the "no follow-ups for similar work" rule andTensors-parity-with-PyTorchperformance directive.Bugs fixed
Output-side rank-1 IOoR (the reported issue) —
outputShape[rank-2]underflows whenrank == 1. Adds rank-0 guard at top + rank-1 case in output reshape returning[_modelDimension].Empty
Trainoverride across 18 language models — Hawk, RecurrentGemma, Mamba/Mamba2, RWKV4/RWKV7, Eagle, Finch, Falcon, GLA, GatedDeltaNet, Griffin, Jamba, Samba, VisionMamba, XLSTM, Zamba, Zamba2 all hadpublic override void Train(...) { }short-circuitingNeuralNetworkBase.Train's tape-routing intoTrainWithTape → CompiledTrainingPlan. Removes the empty overrides so all 18 inherit the working base implementation.Tape-bypass + scalar loop in
GatedRecurrenceForward— Per-cell loop withNumOps.ToDouble/NumOps.FromDouble(a) silently zeroed gradients for_decayParam,_recurrenceGateWeights,_inputGateWeights,_valueProjectionWeightsbecause primitive conversions don't go through the autodiff tape, and (b) ran ~131K boxed conversions per layer per forward at paper-scale. Vectorized via Engine ops: gates, value projection, decay factor,sqrt(1-a²), input contribution all computed as single batch matmuls / element-wise ops; only the genuineh_t = a*h_{t-1} + contribrecurrence stays in the time loop. Forward Step 2's gate loop got the same treatment.AdamOptimizer.Stepallocation storm (OOM site) — 13 transient param-sized tensors per Adam step → ~6.7 GB peak transient on Hawk's 65M-element embedding/LM-head matrices at fp64. Rewrote with in-place ops (4 transients on the engine-op fallback). AddedTryFusedAdamStepspecialized path forT = float | doublethat operates directly onSpan<T>with zero transient tensor allocations, mirroring PyTorch's_fused_adam_. PerfView/dotnet-trace profile showed scalar Adam was 64% of training wall time; SIMD-vectorized viaSystem.Numerics.Vector<T>(AVX2: 4 doubles / 8 floats per pass, AVX-512: 8 / 16 per pass),#if NET6_0_OR_GREATERsince net471 lacksVector<T>(Span<T>)ctor.Adam training divergence on randomly-initialised large models — Adam's first-step bias correction (biasC1 ≈ 0.1, biasC2 ≈ 0.001) creates huge updates on 135M-parameter random-init Hawk. Loss diverged 0.43 → 6.97 over 10 iterations without clipping. Adds PyTorch-style global-norm gradient clipping in
AdamOptimizer.Step's tape path (walks every gradient incontext.Gradientsto computesqrt(Σ_p ‖grad_p‖²), scales in place if exceeding threshold).AdamOptimizerOptionsnow defaultsEnableGradientClipping=truewithMaxGradientNorm=1.0(the canonical PyTorch transformer-training value). Callers needing unclipped behaviour can opt out explicitly.Test results
HawkLanguageModelTestsat paper-scale (vocabSize=256000, modelDim=256, numLayers=4, maxSeqLength=512 → 135M params):The remaining one-test gap is
LossStrictlyDecreasesOnMemorizationTaskwhich runs 100 sequential training iterations at paper-scale (~1.5s/iter on the dev box this was measured on, ≈ 150s vs the test's 180s timeout). Training itself converges with clipping enabled; the wall-time tightness is pure CPU-perf at paper scale and likely passes on CI hardware which is typically faster.Test plan
dotnet test --filter HawkLanguageModelTests.Predict_ShouldBeDeterministic— passes (rank-1 IOoR fix verified)dotnet test --filter HawkLanguageModelTests.Training_ShouldReduceLoss— passes (gradient clipping fix verified, 1m1s)dotnet test --filter HawkLanguageModelTests.MoreData_ShouldNotDegrade— passes (14s)dotnet buildclean on net10.0 and net471🤖 Generated with Claude Code