fix(nn): close 16 pre-existing NN unit-test failures across SSM, embedding, masking, ports, and contracts - #1424
Merged
Merged
Conversation
…nistic projection seed
iscontinuousinput now treats input as continuous when its last axis equals
vocabularysize (the standard [b, t, v] one-hot / probability-distribution
shape), regardless of whether the values happen to look like integer
indices. one-hot tensors carry values strictly in {0, 1} that all pass the
previous value-only check, so they were routed to the index path and the
v scalars per (b, t) position were each gathered as a separate embedding
row — producing rank-(rank+1) output [b, t, v, d] instead of the correct
[b, t, d]. matches pytorch's shape-driven nn.linear vs nn.embedding split.
lazy projection-weight init now defaults to a shape-derived seed (hash of
inputfeatures, embeddingdim, vocabularysize) when no explicit randomseed
is provided. lazy allocation otherwise caused two layers with identical
embedding parameters to diverge on first forward — getparameters() runs
before any forward returns no projection slice, so setparameters on a
sibling layer leaves the receiver's projection null and its own first
forward then initialises with independent secure randomness. the shape
seed restores the determinism contract callers rely on for
setparameters → predict equivalence (rwkv7 / mamba lm tests). explicit
randomseed still wins.
fixes 8 tests across mambalanguagemodeltests and rwkv7languagemodeltests:
predict_2d / predict_3d / predict_double / train_forwardbackwardupdate
on both families, plus rwkv7's predict_deterministicwithsameparameters
that broke when the shape-aware path activated lazy projection init.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
mambablock previously implemented only the inner block transformation (input proj > conv1d > silu > selective scan > gate > output proj). per gu & dao 2023 the canonical mamba design wraps this with a residual: h = h + block(h). without it, repeated block stacks attenuate by ~3 orders of magnitude per layer (observed input range [-0.23, 0.23] decay to [-1e-38, 1e-38] through 4 stacked blocks), collapsing the lm head's logits to zero so the predicted distribution is uniform and the model trains vacuously. the add is shape-aligned: both output3d and input3d carry [batchsize, seqlen, modeldim]. correctness is exercised by the multilayermodel_producesnontrivialoutput test in mambalanguagemodeltests (uses 4 stacked mambablocks with modeldim=16). all 34 mambablock-direct unit tests continue to pass; broader nn unit-test sweep regressed zero tests, falconmamba/eagle/finch lm tests pass in isolation (apparent batched-run flakes are pre-existing parallel-execution artifacts, not caused by this change). reference: gu & dao, "mamba: linear-time sequence modeling with selective state spaces", arxiv:2312.00752, eq. 5 and figure 3. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…line train() routes through trainwithtape > forwardfortraining > layers chain. the base implementation iterates layers directly, but visionmamba's layers list contains ONLY the mambablock stack — the patch embedding, positional encoding, and bidirectional scan all live in custom code in this class's predict() override (separate from the layers list because they reference per-instance state tensors patchprojectionweights, positionalembedding, etc that aren't trainable-layer-shaped). without an override, train(input) flows the raw [b,c,h,w] image directly into mambablock[0]. mambablock's modeldim was sized for _mambainputdim (modeldimension*2 in bidirectional mode = the post-scan feature width), so it expects [b, numpatches, mambainputdim] but receives [b, c, h, w]. mambablock's flatten logic interprets h as seqlen and w as modeldim, producing the [seqlen, w=16] x [modeldim=32, innerdim*2=128] matmul mismatch reported in the failing test. extract a shared private runforward(input) helper that holds the patch>pos>scan>blocks>pool>norm>classifier pipeline, and have both predict (settrainingmode false) and forwardfortraining (settrainingmode true) delegate to it. fixes vision_mambamodeltests.train_forwardbackward update_noerrors. all 16 visionmamba tests pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
the #1382 fail-fast throw rejects `layers: [custom]` paired with `numencoderlayers > 0` because the structural params get silently ignored when a custom layers list is supplied, leaving the model with zero trainable parameters and no optimizer signal (the original #1382 incident). the existing #1317 test asserts the narrower claim that shape-compatible custom layers don't require the user to also include built-in boundary layer types (multihead attention etc) — that claim is preserved by passing numencoderlayers=0 + numdecoderlayers=0 so the auto-build path is fully replaced by the user's list. updated comment documents the constraint. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…s + eager-init for determinism three changes that close 4 layerporttests failures: 1. (test) addlayer port names: align unit tests with the numeric naming (input_0, input_1) that addlayer/concatenatelayer already use and that integrationtests/multiinputporttests already asserts. unit tests previously assumed alphabetic (input_a, input_b) which never matched the code or the integration test contract. 2. (code) diffusionresblock named ports: add inputports override that declares "input" + "time_embed" when timeembeddim > 0 (single "input" otherwise) plus a multi-input forward(dict) override that routes dict-keyed inputs through the existing positional forward(input, timeembed) implementation. semantically-distinct inputs (feature map vs conditioning vector) get descriptive names rather than the generic input_0/input_1 reserved for symmetric n-ary layers. 3. (code) diffusionresblock determinism fix: eagerly materialize all sublayer lazy weights (groupnorms, conv3x3s, time mlp, optional skipconv) at construction time by running a no-grad probe forward through the block. without this, two freshly-constructed blocks start with all sublayer `_weights` as [0,0] placeholders; getparameters returns vectors that exclude those zero-shape weights, setparameters leaves them at [0,0], and the receiver's first forward then lazy-inits with independent random weights — producing divergent outputs and breaking the `block2.setparameters(block1.getparameters()) => block2(x) == block1(x)` contract relied on by checkpoint reload and the multiinputforward_ includestimeconditioning test. memory cost: one probe-forward's worth of activations per block at construction, returned to the pool by resetstate. trivially cheap relative to one training step and dwarfed by the trainable weight memory the eager init unlocks. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…nfiniteoutput opt-out masking layers like alibi (press et al. 2022) and standard causal attention masks emit -infinity at future positions so the downstream softmax assigns exact zero attention weight there. the previous code used numops.minvalue (~-3.4e38 for float), which works for the common forward+softmax path (exp underflows to ~0) but fails any path that adds bias values without going through softmax — residual-summing through masked rows then accumulates a finite ~-3.4e38 penalty instead of cleanly masking, and the computebias_futurepositionsmasked invariant correctly asserts isnegativeinfinity at the bias level. the naive switch to negativeinfinity broke the generated forward_should produceoutput invariant (which assumes finite output for all layers) and the serialize_deserialize_shouldpreservebehavior tolerance check (since math.abs(-inf - -inf) = nan, which fails the < 1e-12 comparison). this commit introduces a clean opt-out so masking layers can legitimately emit ±infinity without disabling the general invariants for other layers: 1. layerpropertyattribute gains a producesnonfiniteoutput bool field (default false). 2. alibi is annotated with [layerproperty(producesnonfiniteoutput = true)]. 3. layertestbase gains a `protected virtual bool expectsfiniteoutput => true;` opt-out; forward_shouldproducefiniteoutput skips the isinfinity check when this is false (the isnan check still runs — nan is always a real bug regardless of layer type). 4. testscaffoldgenerator reads producesnonfiniteoutput and emits a `protected override bool expectsfiniteoutput => false;` on the generated test class for matching layers. 5. serialize_deserialize_shouldpreservebehavior short-circuits via direct equality before the math.abs tolerance check so ±infinity roundtrips correctly (any bit-exact match counts as preserved behavior; the tolerance path still catches near-equal serialization drift for ordinary finite outputs). closes alibi.computebias_futurepositionsmasked. all 33 alibi tests (unit + generated) pass with no regressions in the broader nn unit-test sweep. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…t + industry semantics rope (su et al. 2021, roformer) is a deterministic rotation of query/key vectors using precomputed cosine/sine frequency caches — zero trainable parameters by construction. the previous `assert.true(supportstraining)` contradicted both the documented contract and the actual code: - ilayer.cs:273-283 explicitly states supportstraining returns false for layers that "don't have parameters that need training." - rotarypositionalencodinglayer.cs:60 correctly returns false with the comment "precomputed frequency caches, no trainable parameters." - the layer has no [trainableparameter] attributes and no registertrainableparameter calls. industry semantics research: - pytorch has no `supportstraining` concept directly. the analog for "has trainable params" is `any(p.requires_grad for p in module. parameters())`, which returns false for a parameter-free module (`any` of empty is false). torchtune's rotarypositionalembeddings uses register_buffer for cos/sin caches (non-trainable), matching this layer's design. - keras' layer.trainable defaults to true but is meaningful only as a permission flag for apply_gradients — for a no-weight layer it's vacuously true; count_trainable_params returns 0 either way. - neuralnetworkbase.cs uses the property combined: `layers.where(l => l.supportstraining && l.parametercount > 0)` and `istrainable = l.supportstraining && l.parametercount > 0` — the property is load-bearing only when parametercount > 0, and rope's is always 0. so the test's previous true assertion had no consumer impact, but the semantic claim was wrong. flipped the assertion to false with a citation comment + added an explicit parametercount == 0 assertion to make the contract crisp. fix justified under the global rule's step 6 (test had a contract error, not a code bug). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Contributor
|
Warning Review limit reached
Your plan currently allows 2 reviews/hour. Refill in 52 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughThis PR extends layer test infrastructure to support non-finite outputs, updates several layer implementations with behavioral improvements, and refactors model pipelines. New ChangesLayer Improvements and Test Infrastructure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
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 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/NeuralNetworks/VisionMambaModel.cs (1)
318-322:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove the outer residual add to prevent double-residual behavior.
Line 321 adds an external residual, but
MambaBlock<T>.Forwardnow already appliesinput + block(input). With both, each layer becomes2x + F(x), which distorts the intended stack behavior.Suggested fix
- // Step 4: Pass through Mamba blocks (Layers) with residuals + // Step 4: Pass through Mamba blocks (Layers) var current = scannedInput; for (int i = 0; i < Layers.Count; i++) { - var blockOut = Layers[i].Forward(current); - current = Engine.TensorAdd(current, blockOut); + current = Layers[i].Forward(current); }🤖 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/NeuralNetworks/VisionMambaModel.cs` around lines 318 - 322, The loop currently applies an extra residual add (Engine.TensorAdd) on top of what MambaBlock<T>.Forward already returns, causing double-residual behavior; update the loop in VisionMambaModel so that after calling Layers[i].Forward(current) you assign current to the returned blockOut directly (i.e., remove the Engine.TensorAdd call) so that each layer's output is the block's forward result (which already includes input + block(input)).src/NeuralNetworks/Layers/EmbeddingLayer.cs (1)
840-870: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueShape-based heuristic has a false-positive edge case when
seq_len == vocab_size.The shape-based detection is a sensible heuristic for standard LM inputs
[B, T, V], but it will misclassify index inputs when the sequence length coincidentally equals the vocabulary size:
Input Shape vocab_size Values Detection Correct? [32, 100, 512]512 one-hot continuous ✅ [32, 512]512 indices 0-511 continuous ❌ Users hitting this edge case can set
InputMode = EmbeddingInputMode.Indicesexplicitly. Consider adding a brief inline comment documenting this limitation so future maintainers understand the trade-off.📝 Suggested documentation addition
private bool IsContinuousInput(Tensor<T> input, int vocabularySize) { // Shape-based detection: when the last axis equals the vocabulary size, // the input is one-hot / probability distribution / continuous features // along that axis (the standard LM input shape [B, T, V]). Treating // those V values as token indices instead would mis-rank the output // ([B, T, V, D] vs the correct [B, T, D]) and gather V embedding rows // per (B, T) position. PyTorch's nn.Linear vs nn.Embedding split is // shape-driven for the same reason — index inputs are shape [B, T], // continuous inputs are shape [..., features]. + // + // NOTE: This can produce false positives when seq_len == vocab_size. + // If your index inputs have shape [B, vocab_size], set InputMode = + // EmbeddingInputMode.Indices to bypass auto-detection. if (input.Rank >= 2 && input.Shape[input.Rank - 1] == vocabularySize) { return true; }🤖 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/NeuralNetworks/Layers/EmbeddingLayer.cs` around lines 840 - 870, Add a brief inline comment inside IsContinuousInput explaining the false-positive edge case: when input.Rank >= 2 and the last axis equals vocabularySize (e.g., seq_len == vocab_size) the shape-based heuristic may misclassify index tensors as continuous; document that this is a trade-off of the shape heuristic and that callers can override behavior by setting InputMode = EmbeddingInputMode.Indices. Reference the IsContinuousInput method, the input.Rank / input.Shape[input.Rank - 1] check, and the vocabularySize parameter so future maintainers understand why the shape check exists and how to bypass it.
🤖 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/AiDotNet.Generators/TestScaffoldGenerator.cs`:
- Around line 3243-3249: The ExpectsFiniteOutput override is only emitted in
EmitLayerTestClass, so layers with ProducesNonFiniteOutput still get
finite-output checks in dual-input/multi-input/graph test classes; update all
test-class emitters that can produce tests (including the dual-input emitter,
multi-input emitter, and graph/test graph emitters) to check
layer.ProducesNonFiniteOutput and emit "protected override bool
ExpectsFiniteOutput => false;" when true (same logic currently in
EmitLayerTestClass) so the Forward_ShouldProduceFiniteOutput invariant is
skipped consistently across single-input, dual-input, multi-input, and
graph-generated test classes.
In `@src/Diffusion/NoisePredictors/DiffusionResBlock.cs`:
- Around line 183-202: The comment notes semantic port naming in
DiffusionResBlock's InputPorts (using "input" and "time_embed") differs from
generic indexed names (e.g., AddLayer's "input_0"/"input_1"); update the XML
documentation for the InputPorts property (or add class-level <remarks> on
DiffusionResBlock) to clearly state the port naming strategy: when inputs are
semantically distinct use descriptive names (provide examples "input",
"time_embed") and when inputs are symmetric use indexed names (e.g., "input_0"),
and mention how callers should discover port names at runtime so users know how
to wire inputs correctly.
In `@src/NeuralNetworks/Layers/ALiBiPositionalBiasLayer.cs`:
- Around line 130-141: Condense the long inline comment above the negInf
initialization into a short 2–3 line note and move the detailed numerical
rationale into the layer's XML doc: replace the 12-line block before "T negInf =
NumOps.FromDouble(double.NegativeInfinity);" with a concise comment like "Use
true -Infinity (not MinValue) so softmax sees exp(-inf)=0 exactly on masked
positions; ProducesNonFiniteOutput=true tells tests to skip finite-output
checks." and add the full explanation to the ALiBiPositionalBiasLayer
method/class XML documentation (where other method-level docs live) while
leaving the LayerProperty(ProducesNonFiniteOutput = true) annotation and the
negInf assignment unchanged.
In
`@tests/AiDotNet.Tests/UnitTests/NeuralNetworks/Layers/RotaryPositionalEncodingLayerTests.cs`:
- Around line 19-35: The long 16-line justification above the assertions in
RotaryPositionalEncodingLayerTests should be condensed: replace the verbose
block with a short 2–3 line comment stating that RoPE is
deterministic/parameter-free and therefore SupportsTraining must be false and
ParameterCount == 0; leave the assertions Assert.False(layer.SupportsTraining)
and Assert.Equal(0L, layer.ParameterCount) unchanged and ensure the brief
comment references RoPE/rotary positional encoding and the
ILayer.SupportsTraining contract for clarity.
In
`@tests/AiDotNet.Tests/UnitTests/NeuralNetworks/TransformerCustomLayerValidationIssue1317Tests.cs`:
- Around line 20-31: Replace the verbose 8-line comment above the
TransformerArchitecture<float> instantiation with a concise 2–3 line comment
that states the requirement and the test intent; e.g., mention that
numEncoderLayers and numDecoderLayers must be 0 when passing a custom layers
list (which replaces the auto-built structure) and that this test verifies `#1317`
allowing shape-compatible custom layers. Keep references to
TransformerArchitecture<float>, InputType.OneDimensional,
NeuralNetworkTaskType.SequenceClassification, numEncoderLayers, and
numDecoderLayers so the intent is clear.
---
Outside diff comments:
In `@src/NeuralNetworks/Layers/EmbeddingLayer.cs`:
- Around line 840-870: Add a brief inline comment inside IsContinuousInput
explaining the false-positive edge case: when input.Rank >= 2 and the last axis
equals vocabularySize (e.g., seq_len == vocab_size) the shape-based heuristic
may misclassify index tensors as continuous; document that this is a trade-off
of the shape heuristic and that callers can override behavior by setting
InputMode = EmbeddingInputMode.Indices. Reference the IsContinuousInput method,
the input.Rank / input.Shape[input.Rank - 1] check, and the vocabularySize
parameter so future maintainers understand why the shape check exists and how to
bypass it.
In `@src/NeuralNetworks/VisionMambaModel.cs`:
- Around line 318-322: The loop currently applies an extra residual add
(Engine.TensorAdd) on top of what MambaBlock<T>.Forward already returns, causing
double-residual behavior; update the loop in VisionMambaModel so that after
calling Layers[i].Forward(current) you assign current to the returned blockOut
directly (i.e., remove the Engine.TensorAdd call) so that each layer's output is
the block's forward result (which already includes input + block(input)).
🪄 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: 689e334e-c123-447b-be04-48a58a094aef
📒 Files selected for processing (11)
src/AiDotNet.Generators/TestScaffoldGenerator.cssrc/Attributes/LayerAttributes.cssrc/Diffusion/NoisePredictors/DiffusionResBlock.cssrc/NeuralNetworks/Layers/ALiBiPositionalBiasLayer.cssrc/NeuralNetworks/Layers/EmbeddingLayer.cssrc/NeuralNetworks/Layers/SSM/MambaBlock.cssrc/NeuralNetworks/VisionMambaModel.cstests/AiDotNet.Tests/ModelFamilyTests/Base/LayerTestBase.cstests/AiDotNet.Tests/UnitTests/NeuralNetworks/LayerPortTests.cstests/AiDotNet.Tests/UnitTests/NeuralNetworks/Layers/RotaryPositionalEncodingLayerTests.cstests/AiDotNet.Tests/UnitTests/NeuralNetworks/TransformerCustomLayerValidationIssue1317Tests.cs
… emitters PR #1424 review (CodeRabbit blocking/major): the previous commit added the ProducesNonFiniteOutput parser + LayerProperty field, but only EmitLayerTestClass (single-input layers) actually emitted the `ExpectsFiniteOutput => false` override into the generated test classes. EmitDualInputLayerTestClass, EmitMultiInputLayerTestClass, and EmitGraphLayerTestClass all ignored the flag — so any ±Infinity- emitting layer that ended up with two inputs (e.g. a masking layer fed feature + position), three inputs, or graph-shape inputs silently re-enabled the IsInfinity check and the auto-generated test asserted contrary to the layer's documented contract. Add the same `if (layer.ProducesNonFiniteOutput) … ExpectsFiniteOutput => false` emission to all three peer emitters. Mirrors the rationale on EmitLayerTestClass (Press et al. 2022 / Gu & Dao 2023: −∞ at masked positions is the standard softmax-zero signal) so the flag behaves uniformly regardless of how many inputs the layer takes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…discovery path PR #1424 review (CodeRabbit trivial nit): the InputPorts XML doc said WHY the block uses semantic port names but didn't explain the broader codebase convention or how callers discover the names at runtime. Expand the doc to: - Spell out the two conventions: semantic names ("input", "time_embed") for asymmetric/distinct-meaning inputs vs indexed names ("input_0", "input_1") for symmetric n-ary layers like AddLayer / ConcatenateLayer / MultiplyLayer. - Point at layer.InputPorts as the runtime-discovery surface — each entry exposes Name + Shape, and Forward() keys tensors by Name. Pure doc change; no behavioral or API change. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…XML doc PR #1424 review (CodeRabbit trivial nit): the 12-line inline comment explaining why −Infinity (not NumOps.MinValue) is used for masked positions was correct but hurt code readability around the actual fill loop. Move the full rationale (softmax exactness, FP underflow risk on MinValue, residual-sum gotcha, ProducesNonFiniteOutput annotation contract) into ComputeBias' XML remarks, leaving a brief 3-line pointer inline. No behavioral change. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
PR #1424 review (CodeRabbit trivial nit): the contract correction (SupportsTraining=false for parameter-free RoPE) is correct, but the 16-line inline comment explaining the ILayer contract, PyTorch semantics, and the previous-assertion error history is too verbose for production test code. Condense to the minimum needed for someone reading the test cold: paper cite, contract rule, PyTorch analog. The full history lives in the PR description and git log. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ines PR #1424 review (CodeRabbit trivial nit): the 8-line comment in TransformerCustomLayerValidationIssue1317Tests was correct but overlong. Condense to the essential contract — numEncoderLayers / numDecoderLayers must be 0 when a `layers:` list is provided (#1382), and this test verifies #1317 (shape-compatible custom layers accepted). Drop the "silently ignore"/"vacuous training" elaboration; the PR description and #1382's commit message already cover that history. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ooples
pushed a commit
that referenced
this pull request
May 23, 2026
Brings PRs #1421 (SenseVoice / Paraformer BN→LN), #1424 (NN test failures), and #1430 (audit 2026-05 remediation) into this branch. Two add/add conflicts resolved by accepting HEAD (this branch's post-review versions): - src/NeuralNetworks/Layers/CifAlignmentLayer.cs: keep this branch's `SupportsTraining => false` honest-contract version (review thread PRRT_kwDOKSXUF86EQONV) and the `threshold >= 1.0` guard (PRRT_kwDOKSXUF86EQONS). Master had the earlier `SupportsTraining => true` + `threshold > 0` versions from the cherry-pick of #1421's CIF implementation; the post-review hardening on this branch supersedes them. - tests/AiDotNet.Tests/Performance/SenseVoiceTrainStepProfile.cs: keep this branch's assertion-bearing profile test (PRRT_kwDOKSXUF86EMZ2n / EMZ2t / EMZ3F) plus the `$`-prefix interpolation fix (PRRT_kwDOKSXUF86EQONX) over master's earlier log-only version. The post-review test enforces real budget assertions for ctor / warm-Predict / median-warm-Train / ForwardForTraining phases. No other files conflicted — VERSIONING.md, CONTRIBUTING.md, automated-release.yml, CHANGELOG.md, SECURITY.md, NuGet.config etc. all merged cleanly because both sides edited disjoint regions (or this branch had the same edits already from cherry-picks). Build: dotnet build src/AiDotNet.csproj passes 0 errors / 11477 warnings (warnings unchanged from master tip).
This was referenced Jun 4, 2026
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 16 of 17 pre-existing unit-test failures in
tests/AiDotNet.Tests/UnitTests/NeuralNetworks/with zero regressions in the broader NN sweep (final state: 856 passed, 0 failed, 3 pre-existing skipped). Each commit is a self-contained logical change with an explanatory commit body; commit ordering reflects dependency between fixes (later commits' tests pass only because earlier commits' fixes are in place).Failure baseline (master, this branch's merge base)
17 failing tests across SSM language models, layer-port infrastructure, attention masking, transformer architecture validation, and positional encoding.
Commits (7, all in
fix:/test:lowercase per Conventional Commits)4d6321622fix(embeddinglayer) — shape-aware continuous-input detection + deterministic projection seedIsContinuousInputnow treats[B, T, V](last axis = vocabularySize) as continuous regardless of values, matching PyTorch's shape-drivennn.Linearvsnn.Embeddingsplit. One-hot tensors (values strictly in {0,1}) were being mis-routed to the index path and producing rank-rank+1output. Lazy projection init now uses a shape-derived default seed (HashCode.Combine of dims) so two layers with identical embedding params produce identical projection weights without explicit RandomSeed.Mamba/RWKV7 LM Predict_2D/3D/Double + Train_FwdBwd.bd741716efix(mambablock) — add residual connection per standard Mamba designstate-spaces/mambareference impl, each block wraps inh = h + Block(h). Without it, repeated blocks attenuate ~3 orders of magnitude per layer (observed input range[-0.23, 0.23]decay to[-1e-38, 1e-38]through 4 stacked blocks).MambaLanguageModelTests.MultiLayerModel_ProducesNonTrivialOutput. All 34 MambaBlock-direct unit tests continue to pass.abad6fee4fix(visionmamba) — overrideForwardForTrainingto run patch-embed pipelineTrain()routed through the baseForwardForTrainingwhich iteratesLayersdirectly. VisionMamba'sLayerslist contains only the MambaBlock stack — patch embedding, positional encoding, and bidirectional scan lived in custom code only insidePredict(). Extracted a sharedRunForward(input)helper; bothPredictandForwardForTrainingdelegate to it after their respectiveSetTrainingModecalls.VisionMambaModelTests.Train_ForwardBackwardUpdate_NoErrors.730a8831ftest(TransformerArchitecture custom layers list rejects substrate-correct architectures — relax ValidateCustomLayers to shape-based check #1317) — passnumEncoderLayers=0alongside custom layers listlayers:is provided alongsidenumEncoderLayers > 0produces a 0-trainable-param vacuous-training model. The narrower TransformerArchitecture custom layers list rejects substrate-correct architectures — relax ValidateCustomLayers to shape-based check #1317 claim ("custom shape-compatible layers don't require built-in boundary layer types") is preserved by passingnumEncoderLayers=0 + numDecoderLayers=0so the auto-build path is fully replaced. Kept the TransformerArchitecture layers= override yields 0 trainable parameters → ppl >>uniform #1382 throw.5503f5391fix(layerports) — align test naming + DiffusionResBlock named ports + eager-init for determinismAddLayer/ConcatenateLayeruseinput_0/input_1numeric naming (matchingIntegrationTests/MultiInputPortTests.cs); 4 unit tests updated to match.InputPortsoverride declaring"input"+"time_embed"(whentimeEmbedDim > 0) and aForward(IReadOnlyDictionary)override that routes dict-keyed inputs through the existing positionalForward(input, timeEmbed). Semantically-distinct inputs get descriptive names rather than generic numeric.[0,0]-placeholder weights and SetParameters on a sibling block leaves them at[0,0], so the receiver's first forward lazy-inits with independent random weights — breaking theblock2.SetParameters(block1.GetParameters()) ⇒ block2(x) == block1(x)contract.c2c3c20f9fix(alibi) — true-Infinitymasks +ProducesNonFiniteOutputattribute opt-out (4 files)double.NegativeInfinityat masked positions instead ofNumOps.MinValue. Clean approach so the generatedForward_ShouldProduceFiniteOutputandSerialize_Deserialize_ShouldPreserveBehaviorinvariants don't false-positive on masking layers:LayerPropertyAttributegainsProducesNonFiniteOutputbool field.[LayerProperty(..., ProducesNonFiniteOutput = true)].LayerTestBasegainsprotected virtual bool ExpectsFiniteOutput => true;opt-out;Forward_ShouldProduceFiniteOutputskips IsInfinity check when false (IsNaN check still runs — NaN is always a bug).Serialize_Deserializeshort-circuits via bit-exact equality before the Math.Abs tolerance check so ±∞ roundtrips cleanly.TestScaffoldGeneratorreads the new flag and emits theExpectsFiniteOutput => falseoverride on the generated test class.ALiBi.ComputeBias_FuturePositionsMasked. All 33 ALiBi tests (unit + generated) pass.bc6cd7912test(rope) — assertSupportsTraining == falseper documented contract + PyTorch semanticsany(p.requires_grad for p in parameters())is False for parameter-free modules; AiDotNet'sILayer.SupportsTrainingdocs explicitly say "return false because they don't have parameters that need training";NeuralNetworkBasealways combinesSupportsTraining && ParameterCount > 0making the property load-bearing only when ParameterCount > 0 (which RoPE never is). Test asserted True — wrong by both the documented contract and PyTorch semantics. Fixed per the global rule's step 6 (test had a contract error) with full justification.Test plan
dotnet test --filter "FullyQualifiedName~UnitTests.NeuralNetworks"on net10.0 — 856 passed, 0 failed, 3 skipped (baseline: 17 failed)The one remaining pre-existing failure type from master (3 skipped tests) is unchanged on this branch.
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Improvements