feat(clone): automated, provably-correct cloning for options and layers - #2004
Open
ooples wants to merge 2025 commits into
Open
feat(clone): automated, provably-correct cloning for options and layers#2004ooples wants to merge 2025 commits into
ooples wants to merge 2025 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Contributor
|
Important Review skippedToo many files! This PR contains 2355 files, which is 2255 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2355)
You can disable this status message by setting the |
…k's pinned level count DeepCopy's layer-by-layer path resolved a freshly-constructed destination only when the destination reported IsShapeResolved false AND the source's GetInputShape was entirely positive. Both tests are permanently false for a layer that declares an always-free axis with the -1 sentinel -- TransformerEncoderBlock's sequence length, say -- so the guard skipped exactly the layers that needed it. The destination reached SetParameters with its seven children registered but unmaterialized, reported 2,048 parameters against the source's 3,150,848, and threw. MaterializeDestinationLayer now runs whenever the two counts disagree and, when resolving from the declared shape is not enough, forward-probes the destination with the free axes filled in -- a forward pass is what materializes nested children, because the generated EnsureSubLayersRegistered runs during lazy shape RESOLUTION rather than during parameter materialization. Filling a free axis arbitrarily is sound precisely because a free axis is one whose length sizes no weight. The probe helpers are LayerCloning's, promoted from private to internal rather than copied, so there is one definition of how a lazy layer is driven. Both mismatch messages now name the source and destination input shapes, since a bare count pair does not say which side is wrong. Measured on the models that failed, before -> after: F5TTS DeepCopy threw -> 300,809,216 = 300,809,216 OWSM DeepCopy threw -> 356,463,258 = 356,463,258 Qwen3ASR DeepCopy threw -> 820,232,576 = 820,232,576 CanaryQwen DeepCopy threw -> 1,333,659,520 = 1,333,659,520 Twelve more models fail with an identical signature (Dia, UniAudio, NemotronSpeech, ParlerTTS, FishSpeech, IndexTTS, MARS5TTS, MegaTTS3, FireRedTTS, E2TTS, ParakeetTDT, Voicebox, CSM) and are covered by the same path; the full sweep re-run will confirm them. Separately, ContinuumMemorySystemLayer's numFrequencyLevels was not [LayerState], so the generated factory pinned it to the literal default 3 while reading a five-element updateFrequencies back out of the state bag -- and the constructor's own consistency check between the two rejected the pair, so a HopeNetwork built with five levels could not be cloned at all. An optional argument that another recorded argument is validated against is not optional state. HopeNetwork now rebuilds with five levels; it still mismatches on the first MLP block's input width, which is a separate defect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ckstop LayerCloning guarded reconstruction with clone.ParameterCount != source.ParameterCount. Since the manifest merge, that property throws ParameterLayoutNotReadyException when a slot's shape is deferred -- and a layer that has just been rebuilt has not run a forward pass, so deferred is exactly the state the backstop meets. The comparison could not be evaluated there, let alone trusted. It now compares ParameterLayoutSnapshots slot-wise where both sides publish one, falling back to the scalar test only where they do not. Slot-wise is also strictly stronger where both are resolved: two layers can hold the same number of parameters in a different order, and an equal total hides precisely the reordering that would restore each component's values into its neighbour. Errors now name the first differing slot rather than two totals. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e bug
All four failures were the destination layer never being driven to the source's shape
before parameters were copied into it, reached by four different routes.
AudioVisualCorrespondenceNetwork and AudioVisualEventLocalizationNetwork each carried a
hand-written DeepCopy that rebuilt the model with exactly the arguments their own
CreateNewInstance already passes and then copied parameters in without resolving the
destination. Both overrides are deleted rather than repaired: the base DeepCopy now does
the resolve, so the overrides were the base path minus the one step that matters. (The
EventLocalization one also handed the copy the source's optimizer and loss function, which
its sibling's comment correctly says a copy must not share.)
CRNN legitimately overrides DeepCopy -- its CreateNewInstance handles an ONNX mode -- but
had inlined the same "unresolved AND every axis positive" test that the base used to use,
with the same consequence. MaterializeDestinationLayer is now protected so a subclass that
must override DeepCopy shares the fix instead of reproducing the bug.
BatchNormalization.SetExtraParameters computed its expected length as InputShape[0] * 2 and,
on an unresolved layer, demanded "length -2 (mean + variance for -1 features)" -- a negative
number of values no caller can supply. An empty vector from an equally unresolved source is
two sides agreeing there are no running statistics yet, so it is now accepted; a non-empty
one gets a message that says the shape is unresolved instead of quoting a negative length.
VideoInpaintingBase.ResolveLazyLayerShapes runs underneath ParameterCount, which is a READ,
and its probe threw straight out of it: E2FGVI synthesizes its one-channel hole mask only
when the input arrives with exactly _channels channels, so a probe at any other depth
reached BlendKnownPixels with too few channels and threw "Index 3 is out of range". The
probe is now best-effort; a model that refuses it keeps its layers lazy, which is a smaller
and more honest outcome than making every count, serialization and clone throw.
Measured, before -> after:
AudioVisualCorrespondenceNetwork threw -> 496,898 = 496,898
AudioVisualEventLocalizationNetwork threw -> 6,623,232 = 6,623,232
E2FGVI ParameterCount threw -> 2,250,901 = 2,250,901
CRNN threw -> 0 = 0 (both sides unresolved; the harness cannot
drive CRNN with its 1x4 probe, so this says the
copy no longer throws, not that it carries weights)
HopeNetwork still fails: its rebuilt ContinuumMemorySystemLayer reports 328,960 parameters
against 264,448, because every MLP block comes back 256->256 where the first should be
4->256. That is a separate defect in what the layer records as its input shape.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The existing AIDN080-085 catch AUTHORING mistakes -- you wrote an override, hid a field, forgot 'partial'. This is the first CONSUMPTION rule: you asked the parameter surface the wrong question. A zero count means "not sized yet", not "has no parameters", and the two need opposite handling -- the first is a no-op, the second must park the payload and replay it at materialization. Conflating them is not hypothetical: LayerBase carried a comment saying a zero count "is not a claim that the layer has no parameters; it is the layer saying it does not know yet" directly above a guard that threw on exactly that, rejecting every restore into a deferred layer. That is ~104 CI failures from one comparison, and the comment explaining why it was wrong was already sitting on top of it. A rule is what makes that visible at the keyboard instead of three days later in a shard. Implemented over expressions rather than declarations, since consumption defects live in `count == 0` branches, not in type shapes. ParameterManifest.cs and ParameterComponentRegistry.cs are exempt: they DEFINE the readiness distinction, so they are the two places entitled to compare against zero while implementing it. Entered at Warning per the ADNSHAPE006/007 ladder already used in this repo -- suppressed while the backlog is non-zero, promoted to Error at zero. Recorded in AnalyzerReleases.Unshipped.md, which is the ratchet. Deliberately NOT shipped yet: AIDN086 (component registered without a generator-assigned StableId), AIDN088 (reflection over weight-typed fields outside the manifest) and AIDN089 (assignment to a weight-typed field inside Clone/DeepCopy/Deserialize). AIDN089 in particular must land only after Phase 2 can demonstrably replace hand-rolled clone bodies -- shipping it first would force deletion of code that still carries behaviour, which is precisely the 11c602d mistake re-run through an analyzer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…on-partial types Two changes toward cloning models, and one finding that redirects the approach. NeuralNetworkBase gains WriteConstructionState / GetConstructionState, the exact mirror of LayerBase's, so a model can record the constructor arguments it stores in fields and be rebuilt by calling its own constructor rather than by copying its fields. Reconstruction is what makes a clone trustworthy: re-running the constructor re-derives everything the constructor derives, so a stale derived value cannot cross over. LayerStateGenerator now REPORTS ADN0050 and stops, instead of reporting it and emitting anyway. Emitting `partial class X` against a non-partial declaration is CS0260 -- a raw compiler error pointing into generated source, about code the author never wrote. Every layer is already partial, so that path had never been exercised; 238 non-partial models found it immediately. THE FINDING: the layer generator does not transfer to models, and the gate stays on LayerBase. Widening it compiles, but all 3,616 model constructors then fail ADN0053 on `architecture`. This generator rebuilds from a Dictionary<string, string>, so every constructor argument must survive a round trip through text. Layers take scalars, enums and child layers, which do. A model takes a NeuralNetworkArchitecture<T>, which does not. Cloning does not need it to: a clone holds the LIVE source, so rich arguments can be handed across directly rather than serialized and rebuilt. Deserialization is the case that needs text and already has SerializeNetworkSpecificData. The model factory therefore wants a source-aware signature, not a metadata-only one. The rationale is recorded on DerivesFromStatefulBase, which is kept unused for that next step. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds ModelCloningTests: a cloned model predicts identically to its original, does not share parameter storage with it, and DeepCopy behaves the same way. Three pass. Also deletes the ModelCloning adapter added earlier in this branch. It was written on the plan's premise that models had no automated clone -- taken from #2004's "Not done: Models on NeuralNetworkBase". They do: NeuralNetworkBase.DeepCopy has a copy-on-write fast path and a serialize round-trip fallback, and Clone delegates to it. An extension method could never have been reached anyway, since an instance method always wins. So C1 is not a missing mechanism. It is the existing one failing for deferred and lazy layers, which is what the failure text says: "Cloned model predicts differently from trained model after serialize/deserialize round-trip". That round-trip restores through ParameterComponentRegistry.SetParameters -- the walk this branch fixed to slice by the manifest snapshot rather than by each source's own count. Adding a third clone path would have obscured whether that fix worked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ption sites
Two things, and an honest correction on the first.
THE TESTS ARE NOT PHASE 0 GATES. Phase 0 asks for four characterization tests that FAIL
against today's base, one per restoration bucket, to be used as deletion gates. These four
pass immediately, so they cannot gate anything. They test the REGISTRY's contract, which the
previous commits already made hold, rather than the MODEL-level capability the buckets
describe:
- Bucket E is about a model walking a DIFFERENT enumeration (GetAllLayers(), _convLayers,
_branches) that is not registered at all. My test registers both sources, so the problem
it exists to expose cannot occur.
- Bucket C is about a model invalidating caches after its walk. The registry has no cache,
so the assertion is vacuous there.
- Buckets O and F are already satisfied by the registry as written.
Kept anyway because they are worth having: they pin the 1.1 restore-by-layout ordering
(registration order is deliberately the reverse of stable-ID order), the role-carrying
chunks, and write-through. They are regression tests for what now works. A real gate needs a
specific model from each bucket in e1f8fec's restoration list, with its override removed,
asserting the base covers what the override did. That is still owed.
AIDN087 IS ALREADY EARNING ITS KEEP. Enabling it surfaced seven sites that test
ParameterCount against zero as a readiness test:
ContinualLearningStrategyBase, DecisionTreeAsyncRegressionBase, DecisionTreeRegressionBase,
ModelParameterSources, NeuralNetworkBase, OptimizerBase, StructuredPruningStrategy
NeuralNetworkBase and OptimizerBase being on that list is the significant part: the base that
defines the surface and the optimizer that consumes it both conflate "not sized yet" with
"has none". That is the same defect just fixed in LayerBase, in two more places, and it was
invisible until a rule asked.
Added to WarningsNotAsErrors in both csproj configurations, per the ADNSHAPE006/007 ladder:
suppressed while the backlog is non-zero, promoted to Error at zero. Without this the build
fails outright -- which is also why the first D-F shard measurement produced no test summary:
it died in the build before running anything.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… reverts LayerStateGenerator already infers construction state from a backing field without needing the attribute -- "a constructor argument the layer stores in a field is construction state whether or not anyone wrote the attribute". What it did NOT do was say anything when it could not. Any optional parameter with nothing to read it back from fell through to `UseDefault`, and the generated factory got the literal default with no record anywhere that a value had been dropped. A layer whose clone quietly reverts a hyperparameter looked exactly like one that round-trips perfectly, so the only way to find out which was to sweep every model and read the wreckage -- which is how HopeNetwork surfaced, one model at a time, after the fact. ADN0057 now fires at the pin site. Measured across the library on this build: 63 layers, 80 distinct (layer, parameter) pairs and they are not cosmetic. Among them: CohereDecoderBlock and DbrxDecoderBlock pin layerNormEpsilon, AttentiveTransformerLayer pins epsilon, momentum AND virtualBatchSize, BayesianDenseLayer pins randomSeed, ConvolutionalLayer pins nonlinearityForInit, BatchEnsembleLayer pins rankInitScale, and several layers pin seed. A copy that comes back with a different epsilon or a different init nonlinearity is a different model, and until now nothing in the build said so. Warning rather than Info deliberately. Info is invisible at normal verbosity, and detailed verbosity is not an option here -- it logs all 72k discovered cases and has filled the system drive before. The whole defect being fixed is that this class was invisible, so hiding the report behind a verbosity flag would reproduce it. The full list is reproducible from a build log by grepping ADN0057; each entry is a candidate for the same one-line remedy ContinuumMemorySystemLayer took (give the parameter a backing field), and the diagnostic message names the three field spellings the generator will accept. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ADN0057 (previous commit) reported 63 layers pinning a constructor argument to its literal
default. This gives a backing field to every one whose type the generator's inference can
actually read back -- non-nullable int, double and bool -- following the convention the repo
already used for CohereDecoderBlock._ffnDim:
/// <summary>Construction state: the 'rmsNormEpsilon' the layer was built with.</summary>
private readonly double _rmsNormEpsilon;
...
_rmsNormEpsilon = rmsNormEpsilon; // first statement in the constructor
No attribute is needed: inference already treats a constructor argument stored in a field as
construction state (LayerStateGenerator.cs:282). The field IS the fix.
MEASURED, by flipping ADN0057 to Warning and grepping a full build both times:
distinct (layer, parameter) pairs pinned: 80 -> 38
Confirmed at the generated-code level rather than by the count alone -- the factories now
read the values instead of hardcoding them. Rwkv7Stack had four pinned arguments and all four
changed:
modelDimension: 512 -> state.Int32("modelDimension")
numHeads: 8 -> state.Int32("numHeads")
ffnMultiplier: 4 -> state.Double("ffnMultiplier")
globalIclrMultiplier: 1 -> state.Double("globalIclrMultiplier")
and likewise CohereDecoderBlock/Gemma2DecoderBlock's norm epsilons, MambaBlock's expandFactor,
SoftTreeLayer's temperature and initScale.
ADN0057 is set to Info, not Warning. This project builds with warnings-as-errors, so at
Warning the 80 pinned parameters became 84 build ERRORS and failed the build -- a diagnostic
whose job is to REPORT must not be able to do that. The cost is that Info does not surface at
normal verbosity, and detailed verbosity is not available here (it logs all 72k discovered
cases and has filled the system drive before), so counting them means flipping that one word
to Warning and grepping. The comment at the descriptor says so.
The remaining 38 are deliberately untouched, in two groups. Seven are second activation
functions (GRULayer.recurrentActivation, HighwayLayer.gateActivation,
SqueezeAndExcitationLayer.secondActivation, ...) which need the generator's activation
binding -- it binds only the first scalar and first vector activation -- and not a field.
The rest are nullable or non-round-trippable types (int?, int[]?, T[]?, Tensor<T>?, Random?,
PNAScaler[]?) that inference declines by design: "a nullable backing member DECLINES inference
rather than erroring". Both groups need a generator change, not an edit per layer.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…count NeuralNetworkBase.SetParameters walked `Layers.Where(l => l.ParameterCount > 0)`. A lazily sized layer reports 0 until its shape is known, so the filter SKIPPED precisely the layers a restore exists to populate — the clone kept its random initialization and the restore reported success. That is the model-level half of the clone/serialize weight loss. AIDN087, added two commits ago, named NeuralNetworkBase as one of its seven sites. This is what it was pointing at. The rule found a real defect in the base on its first run, which is the argument for consumption rules over authoring rules: nothing about this line looks wrong until you know that a zero count means "ask me later". Materialization now happens before the walk. That is not a new capability — DCCRN, DeepFilterNet, SAM and ViMUNet each carry a hand-written UpdateParameters whose FIRST statement is ResolveLazyLayerShapes(), for exactly this reason. DCCRN's own comment spells it out: "a bare Layers walk would see 0 parameters, skip every layer, and leave the clone on its own random initialization". Four models independently solving a base-class problem is the signal that it belongs in the base, and moving it here is what makes those overrides deletable under the Phase 3 gate rather than deleted on faith. Also correcting an error from the previous turn: I reported that the base never invalidates weight caches after a restore. It does — InvalidateWeightCachesAfterSuccessfulWeightUpdate followed by OnParametersRestored, at the end of this same method. My grep had maxMatchesPerFile=2 and I read a truncated result as the complete set. Bucket C's capability already exists; the missing one was materialization, not invalidation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
My previous Phase 0 attempt produced four tests that all passed, which gates nothing. This one is validated the only way a gate can be: by reverting the fix and confirming it goes red. LayerBase at f48540b^ : 2 failed / 1 passed LayerBase at HEAD : 3 passed The two that flip are the two halves of the deferred-restore capability, and they have to be separate tests because they fail for opposite reasons: DeferredLayer_AcceptsRestore_InsteadOfRejectingItForAZeroCount — the B1 guard threw "Expected 0 parameters, but got N" at a layer whose zero count meant "not sized yet". DeferredLayer_ParksTheWholePayload_WithoutTruncatingToAPlaceholder — the half that relaxing the guard ALONE would have broken, by dropping the layer into the slicing path and cutting the payload down to placeholder length. The third, ConstructionSizedLayer_StillRejectsAWrongLengthRestore, passes in both states by design. It is the anti-regression clamp: it fails if someone "fixes" the guard by deleting it rather than by making it ask about readiness, which would convert every genuine restore-size bug into silent weight loss. Driven at the LAYER level deliberately. A model-level test runs through whichever hand-written UpdateParameters is still present and proves nothing about the base; DenseLayer(outputSize) declares its input as [-1], so it is genuinely deferred and exercises the base path directly. This is the gate for the DCCRN / DeepFilterNet / SAM / ViMUNet overrides, each of which opens with ResolveLazyLayerShapes(). Under the governing rule those may now be deleted one at a time, each with its own commit and a full model-family run — never two in one commit, which is how 11c602d lost the ability to attribute breakage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
LayerBase.SetParameters threw "Expected 0 parameters, but got 16 (layer BatchNormalizationLayer, own 0, tensors 2, buffers 0, sub-layers 0)" on every deserialize into a layer whose shape comes from its input. The layer HAS two declared tensors; it just cannot size them until it has seen a channel count. EnsureMaterializedForParameterSurface runs first and still cannot size them, so ParameterCount stays 0 and the guard rejected the restore. The comment directly above that guard already said a zero count "is not a claim that the layer has no parameters; it is the layer saying it does not know yet". The guard did not honour it. A layer with declarations but no resolved size now takes the same wholesale path an unregistered layer takes: the vector is parked in Parameters and handed to ApplyResolvedParameters when the shape arrives -- what load_state_dict does for a lazy module in PyTorch, and what Conv1DLayer already depended on. Measured on Serialize_Deserialize_ShouldPreserveBehavior, 173 tests: 45 failures before, 38 after. The "Expected 0 parameters" signature -- 29 of the 45, and the largest single signature in the clone/serialize bucket -- is gone. What remains are genuine count mismatches (8, 16, 75, 98, 576) that name a real disagreement rather than a false zero. Also makes an absent-storage restore refuse rather than silently return in the three field-backed sources. Those accessors have no setter, so a null field meant the values were dropped and the round trip reported success -- a model restored with its declared shape and none of its weights. An empty vector is still nothing to do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…class
TransformerEncoderLayer had hand-written flags and branches to construct cheaply and
allocate later. Twenty-nine other composites allocate their children eagerly the same
way, and copying that machinery into each of them is the wrong shape of fix: the
policy -- WHEN to allocate -- is identical everywhere, and only the wiring differs.
The base now owns the policy:
DeclaredSubLayerShapes() a composite says which child takes which shape
InitializeShapesWithoutAllocating() a constructor brings the layer up without weights
BringUpDeclaredSubLayers() shapes or weights, decided by who is asking
IsConstructingShapesOnly the constructor's counterpart to IsResolvingShapesOnly
DeclaredSubLayerWeightsMaterialized what makes the initializer re-enterable
A composite is left declaring DATA. TransformerEncoderLayer's three private flags become
one, its branch disappears, and what remains is the single fact the base cannot derive:
the children take the embedding width, except the second feed-forward, which consumes the
feed-forward width the first projected up to, and self-attention, which wants a sequence
axis in front.
Shapes are TensorShape, not int[]. TensorShape is the library's shape type and a readonly
struct, so passing it is free -- but its public constructor defensively clones and
ToArray() clones again, so the obvious spelling would pay two copies per child per call to
describe something that never changes. It is built through the internal wrap constructor
(AiDotNet is an InternalsVisibleTo friend) and read back through the internal dimensions
field, which is the same escape hatch ADNPERF001's own message points callers at when it
rejects a throwaway Shape.ToArray(). The declaration is then cached, because
EnsureInitialized deliberately re-enters -- construction resolves shapes, a later caller
allocates -- so a per-call rebuild would allocate the shapes and the list again every time.
Net effect: allocated once per layer instead of once per call, and no clones at all.
Behaviour is unchanged, measured on the same binary as the commit before it:
VideoCLIP family 1 failed / 58 passed (unchanged, = baseline)
LLaVANeXTVideo construction 0.09 s / 19.2 MB (unchanged)
TransformerEncoderLayer round trip EXACT (unchanged)
Emu3 is deliberately NOT addressed here and is not a composite problem: it builds a flat
stack from LayerHelper.CreateDefaultUnifiedGenerationLayers -- MultiHeadAttentionLayer,
DenseLayer, LayerNormalizationLayer -- so its 16.5 GB is a construction-sized LEAF
allocating eagerly, plus a float mirror of every parameter (11.0 GB double[] + 5.5 GB
float[], exactly one float per double). Both are tracked separately.
The other 28 composites are not migrated in this commit; each needs its own wiring and its
own verification, and the base is what they will migrate onto.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tead of resolving them
Second migration onto the base-class deferral, and the first that was not already
deferring, so it exercises the mechanism rather than restating it.
EnsureInitialized called ResolveFromShape on each of the four children behind an
`if (!child.IsShapeResolved)` guard. The guard was redundant -- ResolveFromShape opens
with exactly that check -- and ResolveFromShape ALLOCATES, so a shape walker or a
deferring constructor had no way to ask this block for dimensions without paying for
weights. Declaring the shapes and going through BringUpDeclaredSubLayers gives the
caller that choice, and the block keeps stating the one fact it alone knows: its children
are sized by the block's hidden width, not by the tensor arriving at its input.
The declaration is cached, and its shapes are TensorShape built through the non-copying
wrap, so describing the block costs one allocation for its lifetime rather than four
int[] per call.
Verified on the same binary:
TimeMoE + VideoCLIP families 2 failed / 101 passed
Both failures are pre-existing and were confirmed by re-running with this change stashed:
VideoCLIP NamedLayerActivations_ShouldBeNonEmpty (dual-tower, unrelated), and
TimeMoE Gradients_MatchFiniteDifference, which fails identically at the parent commit
with numeric=0.0000E+000 on 4/4 sampled parameters.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ingle chain
Folding Layers[0..n] in order assumes the model IS that chain. A dual-tower model is
not: VideoCLIP keeps a video stack and a text stack in one Layers list, so the fold
pushed image activations into the text tower's EmbeddingLayer, which refused them --
"element 0 is 0.5967621803283691, which is not a token index in [0, 49408)". The
diagnostic could not answer at all for that model. 902 of the library's models override
PredictCore, so the chain assumption is not safe in general.
The fold stays the fast path, because for a plain chain it is both correct and the
cheapest thing available. When it throws, the model is asked to run itself under the
LayerForwardObserver already wired into LayerBase.Forward, and the activations are read
off what actually executed -- right for a branched model, a custom PredictCore, and a
plain chain alike. Keys keep their existing Layer_{index}_{Type} form, using the position
in Layers when the recorded layer is one of them and call order otherwise, which is the
only index a second tower or a sub-layer has; a layer that runs more than once gets a
suffix instead of overwriting its earlier activation.
Measured on the hand-written neural-network families (99 tests), same binary otherwise:
fold only (before) 7 failed / 92 passed 4 m 36 s
observed Predict always 7 failed / 92 passed 5 m 29 s
fold, then observe 7 failed / 92 passed 4 m 19 s
Replacing the fold outright was tried first and rejected on that measurement: identical
results and slower, because Predict on a generative model is a whole generation loop
where the fold is one pass. The seven remaining failures are the same seven in all three
runs (EchoStateNetwork, FastText, FeedForwardNeuralNetwork, GrokVision, InfoGAN,
Phi3Vision, Transfusion) and are pre-existing; Transfusion and ClaudeVision were
confirmed to time out identically with and without this change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The fold over Layers falls back to an observed forward when it THROWS, which covers a
dual-tower model. It did not cover the other way the fold fails: Layers is empty, the
loop runs zero times, and the method returns an empty dictionary without throwing.
Layers is empty for every model that keeps its real layers somewhere else -- an echo
state network's reservoir and readout are plain matrices, InfoGAN holds a generator, a
discriminator and a Q network. Those models reported no activations at all, and the
caller could not tell "this model exposes none" from "the collection did not work".
Treating an empty result the same as a throw fixes three models with no per-model code:
hand-written neural-network families, NamedLayerActivations, 99 tests
before 7 failed / 92 passed
after 4 failed / 95 passed
now passing: EchoStateNetwork, FeedForwardNeuralNetwork, InfoGAN
The four that remain are two separate problems, and neither is this one:
FastText its OWN Predict throws -- "EmbeddingLayer is in Indices mode but
element 0 is -0.02822100557386875, which is not a token index in
[0, 2000000)". The fixture feeds continuous features to a model whose
contract is token indices, so no activation collector can help.
GrokVision timed out at the 120 s per-test ceiling
Phi3Vision timed out at the 120 s per-test ceiling
Transfusion timed out at the 120 s per-test ceiling
MEASUREMENT NOTE, because the earlier numbers on this were wrong. Runs using --no-build
after building only src/ were reading a STALE AiDotNet.dll: an orphaned testhost from a
killed background run held a lock on tests/.../bin, MSBuild gave up after ten retries,
and the copy silently never happened. That is why an earlier pass reported "the same 7
failures" for a change that in fact fixes three. Separately, those killed runs left five
aidotnet-streaming-pool directories holding 32 GB, which filled the disk to 0 bytes free
and made GrokVision and Phi3Vision fail with IOException instead of their real timeout.
Both were cleaned up and every number above was re-measured with a full build and 32 GB
free, twice.
Regression check, clean build: Serialization + LayerContract + ShapeContract
526 passed / 0 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…h layer Upstream 3ab1699 put "parameters were restored before the first forward, do not overwrite them" inside DenseLayer.EnsureInitialized. It is correct, but it is a rule, and ConvolutionalLayer, FeedForwardLayer and RBMLayer have the same private-latch shape and would each need their own copy of it. The rule now lives in LayerBase.TryAdoptRestoredParameters, with three outcomes: nothing supplied means initialize normally; every tensor supplied AND conforming means keep what arrived and register it; anything else -- a half-delivered set, or shapes that disagree with the resolved geometry -- throws with expected against actual. The middle outcome is why this cannot be a blanket "the tensors exist, so skip". An earlier attempt did exactly that in EnsureInitializedFromInput and swallowed CopyOnWriteCloneTests.FreshDense_RejectsIncompatibleReboundWeightsOnFirstForward: the base can see THAT tensors are present but not whether their shapes are right, because [inputSize, outputSize] is knowledge only the layer has. So the layer declares its expected shapes and the base decides what to do about them -- the same division of labour DeclaredSubLayerShapes already uses in this file for child layers. DenseLayer's 30-line block is deleted and replaced by a DeclaredParameterShapes override carrying the two shapes and their roles. The role doubles as the name in the failure message, so the wording that test pins ("Expected weights [4, 3]" / "received weights [5, 3]") is reproduced from the declaration rather than hand-written per layer. STILL HAND-WRITTEN, AND SHOULD NOT BE: the DeclaredParameterShapes override itself. The fields are already annotated [TrainableParameter(Role = ...)], so the generator knows the field and the role and only lacks the shape; teaching TrainableParameterAttribute a shape expression and emitting DeclaredParameterShapes from it would remove the last hand-written line and cover every lazy layer at once. Tracked. FeedForwardLayer was migrated too and REVERTED: it resizes its own weights when a caller's feature width disagrees (EnsureWeightShapeForInput), so a shape mismatch is a normal state for it rather than a broken restore, and the base's throw fired on healthy layers -- 10+ VideoCLIP failures. Adopting it needs different semantics, not the same declaration. ConvolutionalLayer and RBMLayer are untouched for the same reason: neither has been measured yet. Verified, full build, same binary: CopyOnWriteClone + TransformerLogitsLoss + VideoCLIP family + Serialization + LayerContract + ShapeContract 564 passed / 0 failed Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…declaration
The restored-parameter rule already lived in LayerBase, but the shapes it needs were
reaching it through an override I hand-wrote in DenseLayer. The gap was in the generator,
so that is where this fixes it.
[TrainableParameter] gains a Shape argument -- a comma-separated list of expressions in
the layer's own scope:
[TrainableParameter(Role = PersistentTensorRole.Weights, Shape = "InputShape[0], OutputShape[0]")]
[TrainableParameter(Role = PersistentTensorRole.Biases, Shape = "OutputShape[0]")]
TrainableParameterGenerator emits DeclaredParameterShapes() from those annotations, so no
layer implements it. The generator already knew the field and its role; the shape was the
only missing input. DenseLayer's override is deleted -- 23 lines out, 2 attribute arguments
in -- and any lazy layer now opts in by annotation alone, including ones written later,
which is the case a hand-written check always misses.
An axis written as * becomes -2 and means "this layer adapts this axis". That exists
because FeedForwardLayer was migrated with a fixed first axis and REVERTED: its
EnsureWeightShapeForInput resizes the first axis of its weights when a caller's feature
width disagrees, so a mismatch there is normal operation and the base's throw fired on
healthy layers, taking out 10+ VideoCLIP tests. LayerBase.ShapeMatchesDeclared now skips
-2 axes, so that layer can be annotated as "*, OutputShape[0]" when someone measures it.
The unresolved -1 lazy sentinel keeps its own meaning: any declared axis still negative
(and not -2) makes the generated method return empty, which is the base's signal that the
layer cannot answer yet.
Verified, full build, same binary:
CopyOnWriteClone + TransformerLogitsLoss + VideoCLIP family
+ Serialization + LayerContract + ShapeContract 564 passed / 0 failed
FreshDense_RejectsIncompatibleReboundWeightsOnFirstForward passes against the GENERATED
declaration, including its pinned wording ("Expected weights [4, 3]" / "received weights
[5, 3]"), which the base builds from the role plus the declared shape.
STILL HAND-WRITTEN IN DenseLayer, and the next thing to remove: the one-line
`if (TryAdoptRestoredParameters())` guard inside its own EnsureInitialized. It stays for
now because it covers all seven entry points into that method, where the base's forward
choke point covers only one. Removing it means having the generator emit the whole lazy
allocation from the same Shape annotations -- at which point DenseLayer has no
EnsureInitialized at all. ConvolutionalLayer and RBMLayer are deliberately unannotated
until each is measured.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e FeedForwardLayer
Two things, and the first is why the second was previously impossible.
TryAdoptRestoredParameters decided "was this tensor supplied?" with Length > 0. That is
wrong for a rank-0 placeholder: the product of no dimensions is 1, so an unallocated
tensor reports Length 1 and was counted as delivered. FeedForwardLayer constructs its
fields that way, so the partial-set throw fired during ORDINARY CONSTRUCTION --
"received weights [] and biases []" -- and took out 10+ VideoCLIP tests. DenseLayer never
showed it because its placeholders are [0, 0]. The test is now rank AND length.
That was the real blocker on FeedForwardLayer, not the resizing. With it fixed the layer
annotates cleanly:
[TrainableParameter(Role = Weights, Shape = "*, _outputSize")]
[TrainableParameter(Role = Biases, Shape = "1, _outputSize")]
The * is still needed and still earns its place: EnsureWeightShapeForInput rebuilds the
weights as [actualInputSize, outputSize] when a caller's feature width disagrees, so the
FIRST axis genuinely adapts and pinning it would report a broken restore on a healthy
layer. The second axis and the [1, outputSize] biases are fixed and are checked.
An earlier commit message said this layer could not be annotated because a shape mismatch
is normal for it. Half right: the adapting axis needed the wildcard, but what actually
broke the migration was the rank-0 miscount above, which was a base-class defect that
would have mis-fired on any layer with rank-0 placeholders.
Verified, full build, same binary:
CopyOnWriteClone + TransformerLogitsLoss + VideoCLIP family
+ Serialization + LayerContract + ShapeContract 564 passed / 0 failed
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rotection
ConvolutionalLayer and RBMLayer carried the same defect DenseLayer and FeedForwardLayer
had: a restore binds real tensors into a layer whose private latch is still false, and the
first forward allocates over them. Both now declare their shapes on the fields, so the
generator emits DeclaredParameterShapes and LayerBase.TryAdoptRestoredParameters keeps
what a restore delivered.
ConvolutionalLayer _kernels "OutputDepth, KernelInChannels, KernelSize, KernelSize"
_biases "OutputDepth"
RBMLayer _weights "_hiddenUnits, _visibleUnits"
_visibleBiases "_visibleUnits"
Shapes read off each layer's own allocation, not guessed: ConvolutionalLayer allocates
[OutputDepth, KernelInChannels, KernelSize, KernelSize] and [OutputDepth]; RBMLayer
allocates [_hiddenUnits, _visibleUnits] and [_visibleUnits]. Both leave the visible/input
count at a negative sentinel until resolved, which the generated guard already turns into
an empty declaration.
All four lazy layers in the library are now covered by annotation rather than by any
hand-written rule.
Verified, full build:
CopyOnWriteClone + TransformerLogitsLoss + VideoCLIP family
+ Serialization + LayerContract + ShapeContract 564 passed / 0 failed
ConvolutionalNeuralNetwork + RestrictedBoltzmannMachine
+ ConvNetFusedStemPredict 55 passed / 1 failed
That one failure is RestrictedBoltzmannMachineTests.ParameterGradientAccessor_IsPopulated-
OrExplicitlyUnsupported, and it is PRE-EXISTING: it fails identically with these two files
stashed.
RBMLayer._hiddenBiases is deliberately left unannotated. It carries no [TrainableParameter]
and is picked up by the generator's inverted default instead, so it has nowhere to put a
shape; the declared set is therefore the two annotated tensors, which is sound because the
adopt decision is all-or-nothing across whatever is declared.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
LayerBase.Serialize wrote a bare value count and Deserialize read one, so a layer saved while materialized could not be loaded into a fresh one: the reader had 784 values and a target declaring 16, with no way to tell "wrong data" from "not allocated yet". It rejected the restore. NeuralNetworkBase already solved this -- it calls WriteParameterLayout before the values and ApplyParameterLayout before pouring them in, so the target is sized to exactly what was saved. LayerBase had both methods and used neither. The comment on EnsureMaterializedForParameterSurface even says restore "is told what to allocate instead of having to infer it"; that was true of models only. The standalone layer round trip now does the same, in the same order. Format is distinguished by a negative marker rather than a version field, since a legacy stream begins with a value count that can never be negative -- so older streams still load. Measured on Serialize_Deserialize_ShouldPreserveBehavior, 173 tests: 45 failures at the start of this branch, 38 after deferred layers stopped being rejected, 17 now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The wholesale path puts the entire incoming vector into this layer's own Parameters. That is right for a lazy leaf and wrong for a parent whose values belong to its children: it would accept the restore and then predict differently, which is worse than refusing it. No measurable change in failure count -- 17 either way on Serialize_Deserialize_ShouldPreserveBehavior -- so this is a safety change, not a numbers one. A composite that cannot be sized now fails loudly at the guard instead of silently mis-distributing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… weights A restore into a lazy layer landed correctly and was then destroyed by the layer's own first forward. Measured on a GRU round trip: the parameter vectors were bit-identical immediately after Deserialize, and differed at index 0 after a single Forward. That is why 15 round-trip tests reported "Output[N] differs" while the weights had in fact arrived intact -- the failure was downstream of serialization entirely. EnsureInitializedFromInput runs OnFirstForward when the shape is unresolved, and that initializes weights. It cannot simply be skipped for a restored layer: OnFirstForward is also where derived configuration resolves -- GRULayer sets _inputSize there and EnsureInitialized throws without it. So SetParameters holds the vector when the layer has not resolved its shape, and EnsureInitializedFromInput replays it after initialization has run. Replay is conditional on the materialized surface being the size the restore was validated against; a different size means the layer resolved to something the checkpoint did not describe, and pouring the old values in would be worse than leaving it initialized. Serialize_Deserialize_ShouldPreserveBehavior, 173 tests: 17 failures before, 14 after. Branch total so far: 45 -> 14. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…lean build A clean build was FAILING and the incremental one hid it. dotnet build skipped the offending files and left a stale AiDotNet.dll in the test output, so several test runs measured an assembly that predated the changes under test -- which is why the declared child widths looked inert at restore when they were simply not in the binary. The ten errors were all code the merge brought in that this branch's analyzers reject, which is the same thing the PR exists to remove: ASGDOptimizer, RAdamOptimizer and RpropOptimizer carried hand-written Serialize/Deserialize pairs and were not partial (ADN0060, ADN0061), and two CreateNewInstance overrides only reconstructed their type (ADN0058). Declaring the state and deleting the overrides is what the diagnostics ask for. With the real assembly under test, six Serialize_Deserialize failures became one, and that one was a distinct defect: a block declaring its input as [-1, hiddenSize] publishes exactly that shape, and Deserialize tried to resolve from it, then retried with a batch axis prepended and threw again on the same -1, uncaught. A shape carrying a free axis is not a resolution, so it now leaves the layer lazy for the first forward to size. Layer sweep 220 checked, 0 violations. Serialization 130/130, Parameters 81/81, and the seven newly scaffolded blocks 77/77. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two faults that made declaring SOME children worse than declaring none. The chain returned outright the moment any declaration existed, so a composite that can state a few widths and genuinely cannot state the rest lost the rest. VGGish is the case in point: its own input is [-1, -1], so the convolutions are unknowable, while the two dense layers after the flatten read a width fixed at construction. It now chains only the children the declaration does not cover, and a declared child still contributes its output width to whatever follows. The counting latch stamped its epoch BEFORE doing the work, which conflated "an attempt was made" with "there is nothing left to resolve". A composite consulted once before its declaration was ready -- during construction, when an int field still reads zero -- was locked out of every later attempt in that epoch. Re-entry is now guarded by its own flag and the epoch is stamped only once every child is resolved. VGGish declares the widths it knows, and gains FullyConnectedWidth to name one. Its two dense layers were shape-deferred, contributing nothing to the count while GetParameters materialized them. Layer sweep 220 checked / 0 violations, models 8/8, Serialization 130/130, Parameters 81/81 all unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AutogradFunction.Apply records its tape entry as Output = whatever Forward returned, and the gradient map is keyed by TENSOR IDENTITY. An identity function returns one of its own inputs, so output and input are the same key, and the backward pass found an existing entry and accumulated onto it -- adding the gradient to itself. Megatron's copy region is exactly that shape, and it made the analytical input VJP exactly 2x the finite-difference one on every sampled scalar of ColumnParallelLinear and Stage3ShardedLinear. The input's gradient IS what Backward returned, already carrying whatever the function does to it -- the all-reduce, in this case -- so on a self-aliased key it replaces rather than accumulates. Distinct inputs still accumulate, which is what a function receiving the same tensor twice needs. Returning a fresh tensor from Forward instead was tried and is wrong twice over: it breaks the alias callers rely on, and it disconnected the input from the graph entirely. ColumnParallelLinear, RowParallelLinear, Stage3ShardedLinear and TemporalConv3DLayer now declare scaffold arguments, which is how the defect became visible: 44/44 across the four, autodiff 25/25, layer sweep unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ExpertLayer, MixtureOfExpertsLayer, ParallelStreamsLayer, SpectralNormalization, TimeDistributed and WordCharEmbedding now declare scaffold arguments, taking the sweep from 220 measured layers to 227. Three real defects came out of it. WordCharEmbedding declared no tensor port at all, so a generated test fed random continuous values -- negatives included -- into what are token ids, and it failed on "Packed indices must be non-negative". EmbeddingLayer has declared the same thing all along. Its two embedding projections also read one-hot vocabularies rather than the packed id tensor, which chained sizing had no way to know. TimeDistributed built its output by renting a buffer and filling it with SetSlice per timestep. That records no tape node, so the inner layer's weights were unreachable from the loss and every trainable parameter took a zero gradient -- the layer trained not at all while looking healthy. The steps are now joined through the engine instead. ParallelStreams splits its input between the two streams, so its declared stream layers are half the input width. Layer sweep 227 checked / 0 violations; Serialization 130/130. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… built DeclaredSubLayerShapes returns empty for two unrelated reasons and only HasDeclaredSubLayerStructure tells them apart: the layer declares nothing, or it declares widths and one of the declared children is still null. The second is the ordinary state PART-WAY THROUGH THE CONSTRUCTOR, and registration happens there, so the chain ran at exactly that moment, sized whichever children already existed from the composite's own input, and those wrong widths stuck for the life of the layer. WordCharEmbedding shows it plainly: its word projection reads a one-hot vocabulary, but the chain caught it mid-construction and sized it to 7, the packed id width, against the 10-wide vocabulary its checkpoint holds. Waiting costs nothing, because whoever needs the shapes asks again once the declaration is ready. This also fixed TimeDistributed's two tape-gradient failures, whose inner layer had been sized the same way. Layer sweep 227 checked / 0 violations, Serialization 130/130, Parameters 81/81. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…vectors Three faults, one layer, all of them the kind that leaves a reloaded model predicting differently from the one that was saved. The inner weights were restored only when the forward THREW. After an ordinary forward the layer kept the normalized values, so every pass divided them by the spectral norm again: the weights decayed pass over pass, GetParameters handed back the shrunken numbers, and a checkpoint written after a forward reloaded into a layer that normalized them once more. Restoring in a finally ends that. The power iteration vectors were marked [Scratch], so they were dropped from the checkpoint. They are not scratch: iteration starts from a RANDOM vector, and with a single power iteration by default the norm it lands on depends heavily on where it started -- the reloaded layer computed a different norm and its output moved from 0.664 to 0.355. They are buffers, which is what PyTorch registers them as. Declaring them was not enough on its own, because a buffer is registered only when it is non-null and these were built on the first forward. A freshly constructed layer therefore had no slot to restore into and silently kept its own random vectors. They are now built in the constructor whenever the inner layer can already say how many weights it holds. Serialize round-trip passes. The remaining tape-gradient failure is a separate and deeper matter: normalization is applied by writing values through SetParameters, so the tape never sees the division and the analytical gradient misses what finite differences measure. Layer sweep 227 checked / 0 violations, Serialization 130/130, Parameters 81/81. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tion Matches the design PyTorch's spectral_norm parametrization uses: the inner layer's LIVE weight tensor is divided by sigma and the quotient is bound in its place for the duration of the forward, rather than copying the weights into a Vector, dividing the numbers with NumOps and writing them back through SetParameters. The old form put the division outside the graph entirely. Sigma depends on W, so the analytical gradient was missing that dependence while finite differences measured it, and the layer mutated the module it wraps as a side effect of running. Power iteration is separated out and left outside the gradient: it estimates the singular vectors rather than being a function under differentiation, which is what every reference implementation detaches. It refines only while training, so inference is reproducible, and the originals are rebound in a finally so the wrapped layer keeps the weights it came with. Ten of the layer's eleven generated tests pass, including the serialize round trip. The gradient check still disagrees, 0.311 analytical against 0.483 numerical, with u and v frozen by the harness's eval mode -- so the remaining suspect is the VJP of a broadcast divide with respect to its broadcast denominator, which has to sum over the broadcast axes. Layer sweep 227 checked / 0 violations, Serialization 130/130. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…training Two faults left after moving the normalization onto the tape. Dividing by a broadcast sigma gave the wrong gradient. Expressing the same thing as a multiply by its reciprocal makes the analytical gradient agree with finite differences, which points at the backward of a broadcast divide with respect to its broadcast denominator -- it has to sum over the broadcast axes. Worth chasing on its own; the layer takes the form that is demonstrably correct. Power iteration starts at a RANDOM vector, and u^T W v on a random pair is an arbitrary bilinear form, as easily near zero or negative as not, so dividing by it normalizes nothing. The vectors are frozen outside training on purpose, which means a layer used for inference before it ever trained divided its weights by that arbitrary number: output swung between -1.99 and 3.15 across a serialize round trip on nothing but which random pair each instance drew. They are now refined once at construction, so sigma is a real estimate from the start whatever mode the layer runs in. PyTorch leaves them random until the first training forward and inherits the same hole. All eleven of the layer's generated tests pass. Layer sweep 227 checked / 0 violations, Serialization 130/130, Parameters 81/81. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tribution The previous commit blamed the backward of a broadcast divide for the gradient mismatch. That was wrong, and the reasoning behind it was confounded: sigma was still being computed from unseeded random iteration vectors at the time, so swapping the divide for a reciprocal and a multiply changed two things at once. Divide plus seeding passes all eleven tests, so the divide backward was never at fault and no Tensors change is warranted. The whole defect was the random vectors, which the following commit already fixed. The explicit Broadcast* spelling is also the older one. AiDotNet.Tensors #919 made the element-wise operators broadcast on their own, and the pinned 0.128.0 contains it, so TensorAdd and TensorDivide say the same thing more plainly. Layer sweep 227 checked / 0 violations, Serialization 130/130. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Addresses the two CodeQL findings on PR #2004 (code-scanning 20876 and 20877, undisposed Tensor<T> at SileroVad.cs:447 and :510), and a larger defect found at the same two lines. Both frame loops built a per-frame tensor and then assigned the samples into frameTensor.ToVector(). That method ALLOCATES A NEW VECTOR AND COPIES (Tensor.cs:662), so the write landed in a throwaway and the tensor handed to the model stayed all zeros: GetFrameProbabilities and DetectSpeechSegments scored every frame as silence, and their output did not depend on the audio at all. This is not a new diagnosis. PreprocessAudio in the same file already carries a comment describing exactly this defect - "the bug that made the conv frontend see a zero signal and emit a constant 0.5 for every input" - and writes through Data.Span instead. These two loops were surviving instances of it, and they had no test coverage, which is why they survived. Both now use the same Data.Span idiom. The disposal CodeQL asked for is applied as `using` per frame. It is safe here because PreprocessAudio allocates its own result tensor, so nothing downstream retains the per-frame tensor once GetSpeechProbability returns. Two tests. FrameProbabilities_DependOnTheAudio asserts the property a caller cares about rather than the mechanism, and restoring the ToVector() write fails it with "every frame probability was identical for silence and for a loud signal". The determinism test still passes under that mutation - zeros are perfectly deterministic - which is exactly why determinism alone could never have caught this. Verified: builds clean on net10.0. New tests 2/2 and mutation-proven. VoiceActivity + SileroVad 34 passed, 1 skipped, 1 failed - TrainingError_ShouldNotExceedTestError, which fails identically with this change stashed and is therefore pre-existing.
The build gate reported "Generated test classes are not covered by any shard filter: RowParallelLinearTests". Declaring RowParallelLinear for the scaffold generator produced a test class under a prefix no shard selects, and an unselected class does not fail -- it silently never runs, which is what check-shard-coverage.ps1 exists to catch. Row joins the shard that already carries RoM, Robu, Rod, Roo and Rot.
… them CodeQL flagged two undisposed Tensor<T> locals in DetectSpeechSegments and GetFrameProbabilities. Reading them showed a second defect at the same lines: both wrote the frame samples into the result of frameTensor.ToVector(), which returns a COPY. The tensor stayed all zeros, so both methods scored silence no matter what audio the caller passed. PreprocessAudio in this same file already documents that exact bug and writes through Data.Span instead; these two sites were never updated. They now do the same, and each frame tensor is scoped with using so the per-frame allocations are released rather than left to the finalizer.
A graph layer's adjacency matrix is [numNodes, numNodes] for whatever graph the caller handed in,
so declaring it [FittedParameter] put a caller-sized tensor in the flat parameter vector and made
ParameterCount change under a forward pass. GraphGenerationModel was the visible casualty: its
optimizer built an update from the trainable view while the restore read the full one, and the two
disagreed by exactly the adjacency's width.
[FittedParameter(InputSized = true)] separates "persist this" from "count this". Such a member
registers as a buffer -- so it is written and read by name in the serialized buffer block -- but is
never declared as a parameter component, leaving ParameterCount determined by construction alone.
Three places had to agree for that to hold, and each was its own silent hole:
- The generated AppendDeclaredParameterComponents omits it. On its own this achieved nothing,
because GetOrderedParameterComponents sweeps every REGISTERED buffer and re-declares whatever
the declaration left out. The base now skips the new InputSizedState role there, which is what
actually keeps the slot out.
- ReadRegisteredBuffers skipped any name with no live tensor, so a member that only exists once
the caller supplies it could never be restored -- the payload had nowhere to land. Buffers are
now rebuilt from the saved shape and installed through a generated name-to-field map, because
registering a tensor without writing the field leaves the layer computing with the old one.
- LayerCloning copied GetTrainableParameters, which is the optimizer view and holds no buffers at
all; clones only ever carried them by riding the vector. Buffer values are now copied by name.
RegisterBuffer's default state role is Buffer, so registering a restored slot from the call site
promoted it back into the vector. The generated map registers under the member's own role instead,
and ApplyParameterLayout routes through the same map rather than allocating with the default.
Readonly buffer fields are left out of the map: they are assigned once by the constructor, are
therefore always live for the write-through path, and cannot be assigned anyway.
The sixteen tensors that carry graph structure -- adjacency on ten layers, GraphTransformer's structural bias, MessagePassing's and EdgeConditional's edge features, DiffusionConv's laplacian, mass matrix and eigenvectors -- are all sized by the caller's graph, not by construction. They now say so with [FittedParameter(InputSized = true)], so they still serialize and deep-copy while ParameterCount stops moving when a graph arrives. GraphGenerationModel had two more of the same mistake: _autoAdjacencyMatrix was [TrainableParameter]. It is the self-loop identity matrix EnsureAdjacencyMatrix derives when a caller predicts without supplying a graph, and it is rebuilt whenever the node count changes, so a checkpoint has nothing to carry. Declaring it trainable handed a graph to the optimizer. It is [Scratch]. Train built its gradient vector by walking ITrainableLayer.GetTrainableParameters and pairing the result with GetParameters(). Those are two different views and the walk was a second ordering written by hand -- exactly what FillParameterGradients exists to prevent, and its own remarks warn that such a walk "will drift the first time either side gains a member". It did. The tape is now published through the base surface, which scatters each layer by mirroring FillParameters. The hand-written GetParameterGradients override went with it: it appended _meanWeights and _logVarWeights to base.GetParameterGradients(), which already folds every tensor GetExtraTrainableTensors declares, so the extras were counted twice. Measured on GraphGenerationModelTests: 25 passed, 7 failed, and origin/master run under the same filter fails those same 7 with the same pre-existing NaN in the VGAE backward. The perf census fixture that the CI gate reported as failed now records status ok at 2,624 parameters.
… cache Two corrections to the input-sized work. ParameterSlotRole.InputSizedState was inserted after LearnedState, which renumbered every role after it. Appended instead, so existing ordinals keep their meaning. LinkPredictionModel's _cachedAdjacencyMatrix was [Buffer] while GraphClassificationModel and NodeClassificationModel mark the identically named field for the identical purpose [Scratch]. It only started to matter when the class became partial and the model-level generator began reading it: a nullable buffer with no initializer is derived as fit-produced, so the registry refused to report parameters until the model was "fit", and the caller's [numNodes, numNodes] graph landed in GetParameters() while ParameterCount left it out. It caches what the caller supplied, and #1593 made supplying it the contract, so it joins its two siblings as [Scratch]. Measured: the thirteen graph-layer and graph-task fixtures run 394 tests, 380 passed and 7 failed, and origin/master under the same filter fails the same 7 with the same pre-existing NaN. Layer surface sweep 227 checked / 0 violations; Serialization 130/130; Parameters 81/81.
…land The collaborator fixed the two CodeQL findings in SileroVad while I was fixing the same two, and reached the same diagnosis: the samples were being written into the ToVector() copy, so both frame loops handed the model an all-zero tensor. The code is identical on both sides; theirs is kept because it also adds SileroVadFramePayloadTests, which pins the property a caller cares about (probabilities depend on the audio) and is mutation-proven against restoring the ToVector() write. Mine had no test.
added 7 commits
August 20, 2026 19:27
# Conflicts: # src/NeuralNetworks/SyntheticData/TimeGANGenerator.cs
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
3 participants
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
Completes the automated cloning/serialization work for #1993 and removes the remaining concrete model/layer lifecycle plumbing. The acceptance baseline is merged
mastercommit1f3bc962ca(#2035); the current PR head isde55b75e05.Concrete model and layer authors now declare configuration, parameters, buffers, fitted state, and scratch state. Construction replay, cloning, parameter discovery, and serialization are supplied by generators and shared base classes.
Architecture
Clone,DeepCopy,CreateNewInstance,Serialize,Deserialize, network-specific serializer, and extra-parameter ownership methods from governed model/layer types.ADN0063, promoted to an error in this repository, so concrete lifecycle overrides cannot grow back. Abstract/shared bases remain the implementation boundary; optimizers and distributions are outside this model/layer rule.[Scratch]graphs and supports model-owned trainableVector<T>state.Timeout and regression hardening
FluxInpaintingModelandSiDDiTModelgenuinely exceeded 120 seconds alone at paper scale. Their model-family tests now inject small real predictor/VAE graphs, retain production defaults, and run in the normal PR lane instead of hiding lifecycle checks behindHeavyTimeout.Automated CI proof
The workflow now exposes a dedicated CI Regression Proof job and uploads a compact
test-outcome-ledger-<sha>artifact. It reports:Current run: https://github.com/ooples/AiDotNet/actions/runs/32599415024 (queued for GitHub-hosted capacity). This PR is not declared merge-ready until that run publishes its final proof.
Local verification at
de55b75e05net10.0test-project build: 0 errors.ADN0063is active as an error.git diff --check: no whitespace errors.No assertion was weakened, tolerance loosened, or timeout increased to obtain these results.
Reviewer focus
src/AiDotNet.Generators/CloneAutomationAnalyzer.cssrc/AiDotNet.Generators/ClonePlanGenerator.cssrc/AiDotNet.Generators/ModelParameterGenerator.cssrc/Models/Parameters/GeneratedParameterDiscovery.cs.github/scripts/test-regression-analysis.ps1.github/workflows/sonarcloud.yml