feat: PyTorch-style lazy shape inference for ~50 layers + backbone refactor (#1209)

[ooples]

Summary

Implements issue #1209: PyTorch LazyConv2d/LazyLinear-style shape inference across the layer stack, plus the architectural follow-ups that this unblocks (backbone refactor, test-generator routing fix, related issue #1136 NN-Classic 122-test cluster).

Architectural primitives

• LayerBase<T> lazy primitives: IsShapeResolved, OnFirstForward(input), ResolveShapes(inputShape, outputShape), EnsureInitializedFromInput(input). Shape resolution and weight allocation deferred to first forward.
• NeuralNetworkArchitecture<T> accepts -1 sentinel for dynamic spatial dims; new CreateDynamicSpatial(inputType, taskType, channels, outputSize) factory.
• IFeatureMapProvider<T> mixin interface for backbones that produce multi-scale feature pyramids.
• IDetectionBackbone<T> = INeuralNetworkModel<T> + IFeatureMapProvider<T> (marker interface that decouples detection consumers from BackboneBase's class hierarchy).
• ONNX RequireResolved() guard before export + CompiledModelHost cache key widened with resolved-input-shape hash.
• TestScaffoldGenerator dynamic-vision template (uses CreateDynamicSpatial for vision models).

Layer migrations (~50 types)

Spatial-rigid: ConvolutionalLayer, DeconvolutionalLayer, DilatedConv, SeparableConv, DepthwiseSeparableConv, LocallyConnected, Conv3D, MaxPool, MaxPool3D, AvgPool, GlobalPool, AdaptiveAvgPool, PatchEmbed, PixelShuffle, Cropping, Padding, Upsampling, Upsample3D, SpatialTransformer, SpatialPooler, DiffusionConv, SpiralConv.

Shape-passthrough: Residual, Flatten, Reshape, Transpose, Mean, PReLU, Masking, LogVariance, Activation, Split, RepParameterization, GaussianNoise, BatchNorm, LayerNorm.

Feature-rigid (largest call-site impact):

• DenseLayer: ~2200 callsites across 97 files
• MultiHeadAttentionLayer: ~295 callsites across 21 files
• FullyConnectedLayer: ~298 callsites across 55 files
• FeedForwardLayer: ~41 callsites
• AttentionLayer: ~16 callsites
• Plus: MemoryRead/Write, Capsule, GLU + variants (SwiGLU/GeGLU/ReGLU/BilinearGLU), GatedLinearUnit, GatedFeatureLearningUnit, Decoder.

Bulk migrations done with paren-aware Python migrators (handle nested parens, multi-line calls, named args, fully-qualified types) with heuristic skip-already-migrated detection.

Backbones (#151–#153, fixes #1136)

• BackboneBase<T> now : NeuralNetworkBase<T>, IDetectionBackbone<T> (was : ModelBase<T,Tensor<T>,Tensor<T>>). All backbones (ResNet, CSPDarknet, EfficientNet, SwinTransformer) now satisfy INeuralNetworkModel<T>.
• ConvUtils rewritten: Conv2D<T> / Dense<T> / MultiHeadSelfAttention<T> are thin delegating wrappers around stock ConvolutionalLayer<T> / DenseLayer<T> / MultiHeadAttentionLayer<T>. File slimmed from 550 → 180 lines. Backbones keep their existing API (no changes to ResNet/CSPDarknet/EfficientNet/SwinTransformer source).
• TestScaffoldGenerator routing now compatible with backbones (priority-18 UsesTensorInput → NeuralNetwork produces correct scaffolds). The temporary BackboneBase exclusion was removed once the inheritance landed.

This resolves issue #1136 (NN-Classic 122 test failures rooted in backbones being routed to NeuralNetworkModelTestBase without implementing INeuralNetworkModel<T>).

LayerHelper (#148)

CreateDefaultViTLayers cleaned (drops dead imageHeight/imageWidth/imageChannels); 9 callers in VisionLanguage/Encoders/* updated. Remaining ~60 helpers have dead-but-harmless params left intact — bulk-cleanup attempted but body references to dropped tracking variables are too brittle for safe en-masse removal; tracked as follow-up.

Test plan

• dotnet build src/AiDotNet.csproj -f net10.0 — 0 errors
• dotnet build src/AiDotNet.csproj -f net471 — 0 errors
• dotnet build tests/AiDotNet.Tests/AiDotNetTests.csproj -f net10.0 — 0 errors
• dotnet build tests/AiDotNet.Tests/AiDotNetTests.csproj -f net471 — 0 errors
• Run lazy-shape regression suite (added prior session): LayerShapeResolutionTests, ArchitectureDynamicSpatialTests, BackboneVariableShapeTests, EndToEndDetectionShapeRobustness
• Run NN-Classic shard to confirm perf: fix 5 cancelled CI jobs — Diffusion models OOM/timeout from eager weight allocation #1136 122-test cluster resolved
• Run Generated test shards (ResNet/CSPDarknet/EfficientNet/SwinTransformer)
• Run full Diffusion shard (no regressions)

Out of scope (tracked separately)

• Symbolic-axis ONNX export with dynamic_axes (ONNX export: symbolic-axis support for dynamic input shapes #1211)
• Full RNN/Transformer end-to-end lazy migration (LSTM/GRU/ConvLSTM/MambaBlock) — current PR makes them lazy via constituent Dense/Attention but their explicit ctors stay (Lazy input-feature ctors for LSTM / GRU / Transformer layers #1212)
• Composite/specialized layers (MoE, RBM, ConditionalRandomField) — internal Dense/Conv usages get lazy automatically (Lazy ctors for composite/specialized layers (MoE, Capsule, RBM, CRF, etc.) #1213)
• Removing the architecture requirement from NeuralNetworkBase (Make NeuralNetworkArchitecture optional in NeuralNetworkBase #1214)
• Remaining LayerHelper helper-builder dead-param cleanup (60+ helpers)

Closes #1136

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added detection backbone & feature-map provider contracts and a layer shape-resolution property.

• Refactor

  • Widespread layer initialization updates so models resolve shapes/parameters at construction for immediate parameter/export availability.
  • Many layers/attention blocks now initialize eagerly to make parameter counts and exports deterministic.

• Stability

  • Stricter input/shape validation, clearer unsupported-operation errors, and improved deep-copy/clone reliability across detection and VAE components.

[coderabbitai]

Actionable comments posted: 20

♻️ Duplicate comments (5)

src/Diffusion/Conditioning/CLIPTextConditioner.cs (1)

298-335: ⚠️ Potential issue | 🔴 Critical

Blocking: replace the linearized “attention/MLP” surrogate with the real CLIP block.

This loop still substitutes both self-attention and the MLP with single linear projections. That keeps the conditioner in the non-production “teaching-grade” state explicitly forbidden for src/**. Route each layer through the existing production attention + feed-forward primitives instead of LinearProject(...), and pass attentionMask through that path so masking, residual order, and block semantics match the real model.

As per coding guidelines, “Production Readiness (CRITICAL - Flag as BLOCKING) … Simplified implementations … require immediate fix.”

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Diffusion/Conditioning/CLIPTextConditioner.cs` around lines 298 - 335,
The loop inside ApplyTransformerLayers currently uses LinearProject for both
attention and MLP; replace those surrogates with the real CLIP transformer block
flow: call the existing attention primitive (the production self-attention
method) with the correct slice of TransformerWeights, headDim/NumHeads and pass
the attentionMask through, apply residual + LayerNorm semantics exactly as in
the real block (use CopyVector/CopyVector for residuals, LayerNorm for pre/post
norms), then call the production feed-forward (MLP) primitive instead of
LinearProject using the correct MLP weight offsets from TransformerWeights;
ensure you use the same lnGamma/lnBeta extraction already present, preserve
residual connection ordering (residual -> LayerNorm -> Attention -> add residual
-> LayerNorm -> MLP -> add residual), and remove the two LinearProject calls so
ApplyTransformerLayers, LinearProject, TransformerWeights, LayerNorm,
AddVectors, CopyVector and attentionMask are correctly wired to the production
attention + feed-forward primitives.

src/Diffusion/VAE/VAEDecoder.cs (1)

199-258: ⚠️ Potential issue | 🔴 Critical

Resolve the lazy decoder convolutions in the constructor.

_postQuantConv, _inputConv, and _outputConv are now created lazily, but unlike the encoder equivalents they are never ResolveFromShape(...)'d. A freshly constructed VAEDecoder<T> can therefore still expose incomplete parameter state until Forward() runs, which breaks GetParameters(), SetParameters(), and checkpoint round-trips on new instances.

🔧 Suggested fix

         _postQuantConv = new ConvolutionalLayer<T>(
             outputDepth: latentChannels,
             kernelSize: 1,
             stride: 1,
             padding: 0,
             activationFunction: new IdentityActivation<T>());
+        _postQuantConv.ResolveFromShape(new[] { 1, latentChannels, _bottleneckSize, _bottleneckSize });

         // Input convolution to expand latent to decoder channels
         int lastChannels = baseChannels * _channelMults[^1];
         _inputConv = new ConvolutionalLayer<T>(
             outputDepth: lastChannels,
             kernelSize: 3,
             stride: 1,
             padding: 1,
             activationFunction: new IdentityActivation<T>());
+        _inputConv.ResolveFromShape(new[] { 1, latentChannels, _bottleneckSize, _bottleneckSize });
@@
         _outputConv = new ConvolutionalLayer<T>(
             outputDepth: outputChannels,
             kernelSize: 3,
             stride: 1,
             padding: 1,
             activationFunction: new IdentityActivation<T>());
+        _outputConv.ResolveFromShape(new[] { 1, baseChannels, _outputSpatialSize, _outputSpatialSize });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Diffusion/VAE/VAEDecoder.cs` around lines 199 - 258, The lazy decoder
convolutions (_postQuantConv, _inputConv, _outputConv) are instantiated but
never ResolveFromShape(...)'d, leaving parameter lists incomplete until
Forward() runs; to fix, call ResolveFromShape(...) on each of those layers in
the VAEDecoder<T> constructor using the correct tensor shapes (e.g. use
latentChannels/_bottleneckSize for _postQuantConv, lastChannels and
_bottleneckSize for _inputConv, and outputChannels with final spatial size for
_outputConv) so their parameters are initialized immediately and
GetParameters()/SetParameters()/checkpointing work without needing Forward().

src/Diffusion/NoisePredictors/NoisePredictorBase.cs (1)

657-669: ⚠️ Potential issue | 🔴 Critical

Blocking: Train() still routes through a runtime stub.

Train(...) calls ComputeGradients(...), and the base implementation now just throws. That turns a missing override into a production-time failure instead of a compile-time contract. Make this abstract, or provide a real base implementation that can collect trainables and delegate to tape. As per coding guidelines, "Every PR must contain production-ready code" and "All methods have complete, production-ready implementations."

Proposed fix

-public virtual Vector<T> ComputeGradients(Tensor<T> input, Tensor<T> target, ILossFunction<T>? lossFunction = null)
-{
-    if (input == null)
-        throw new ArgumentNullException(nameof(input));
-    if (target == null)
-        throw new ArgumentNullException(nameof(target));
-
-    throw new NotSupportedException(
-        $"{GetType().Name} does not implement ComputeGradients. " +
-        "Override this method on the concrete predictor and route through " +
-        "ComputeGradientsWithTape with the predictor's collected trainable tensors. " +
-        "AiDotNet has no per-layer Backward; autodiff goes through the GradientTape " +
-        "(see DiffusionModelBase.ComputeGradients for the canonical pattern).");
-}
+public abstract Vector<T> ComputeGradients(
+    Tensor<T> input,
+    Tensor<T> target,
+    ILossFunction<T>? lossFunction = null);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Diffusion/NoisePredictors/NoisePredictorBase.cs` around lines 657 - 669,
The base method ComputeGradients in NoisePredictorBase currently throws
NotSupportedException causing runtime failures when Train calls it; either make
ComputeGradients abstract to force concrete predictors to implement it, or
implement a production-ready default that gathers the predictor's trainable
tensors and delegates to ComputeGradientsWithTape (mirror the pattern in
DiffusionModelBase.ComputeGradients): update NoisePredictorBase.ComputeGradients
to be abstract or to call the predictor's collected trainable tensors and invoke
ComputeGradientsWithTape(input, target, trainables, lossFunction) so Train no
longer depends on a runtime stub.

src/Diffusion/VAE/VAEModelBase.cs (1)

576-592: ⚠️ Potential issue | 🔴 Critical

Blocking: the new backprop hook is still a no-op placeholder.

The “primary” exact-gradient path depends on BackpropagateLossGradient(...), but the base implementation intentionally does nothing and then relies on the SPSA fallback. That is still a stub in the core training flow. Make this abstract for subclasses that support exact gradients, or route the base implementation through a real tape-based path instead of silently degrading. As per coding guidelines, "Every PR must contain production-ready code" and "Stubs/Placeholders ... are blocking issues requiring immediate fix."

Proposed fix

-protected virtual void BackpropagateLossGradient(Tensor<T> lossGradient)
-{
-    // Default no-op. Concrete VAEs that maintain layer-level state should override.
-}
+protected abstract void BackpropagateLossGradient(Tensor<T> lossGradient);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Diffusion/VAE/VAEModelBase.cs` around lines 576 - 592, The
BackpropagateLossGradient(Tensor<T> lossGradient) method in VAEModelBase is a
no-op placeholder causing silent fallback to SPSA; change its signature to be
abstract (declare protected abstract void BackpropagateLossGradient(Tensor<T>
lossGradient)) so concrete VAE implementations must provide an exact-gradient
path, and update all subclasses of VAEModelBase to implement
BackpropagateLossGradient (calling their layer.Backward(grad) or tape-based
backward logic) to restore the primary exact-gradient flow; ensure the base
class is made abstract if needed and adjust any callers/tests to account for the
now-required implementation.

src/ComputerVision/Detection/Backbones/ConvUtils.cs (1)

54-59: ⚠️ Potential issue | 🟠 Major

Conv2D<T> still drops the no-bias contract used by existing backbone code.

Hard-wiring a bias here changes parameter counts and serialized layouts for every former useBias:false call site. That is a silent checkpoint-compat break and changes numerics in conv blocks that were intentionally bias-free. Either plumb bias control through to ConvolutionalLayer<T>, or version/upgrade the serialization format instead of changing the contract in place.

#!/bin/bash
set -euo pipefail

# Inspect whether the wrapped convolution layer supports configurable bias.
fd -a '^ConvolutionalLayer\.cs$' src | while read -r file; do
  echo "--- $file ---"
  rg -n -C3 'class ConvolutionalLayer|Bias|bias|useBias' "$file"
done

# Show the wrapper surface and current detection call sites now forced through the bias-always-on ctor.
echo
echo "=== Conv2D wrapper and call sites ==="
rg -n -C2 'class Conv2D<T>|new Conv2D<' src/ComputerVision/Detection

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ComputerVision/Detection/Backbones/ConvUtils.cs` around lines 54 - 59,
The Conv2D constructor currently forces a bias term, breaking the prior
useBias=false contract and altering params/serialization; update Conv2D (the
Conv2D(int inChannels, int outChannels, int kernelSize, int stride = 1, int
padding = 0) constructor) to accept a bool useBias parameter and propagate it
into the underlying ConvolutionalLayer<T> (or, if ConvolutionalLayer<T> lacks
bias support, add a useBias flag to ConvolutionalLayer<T> and its ctor and
serialization logic), ensure call sites creating new Conv2D<> are updated or
overloaded to preserve backward compatibility, and adjust
serialization/versioning in the ConvolutionalLayer<T> (and any
Serialize/Deserialize methods) to maintain checkpoint compatibility when useBias
changes.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/AiDotNet.Generators/TestScaffoldGenerator.cs`:
- Around line 1130-1156: The XML doc for IsExactlyArchitecture got orphaned
because IsPatchVisionModel was inserted above it; move the entire
IsPatchVisionModel method so it appears immediately after the
IsExactlyArchitecture method (so the existing XML comment attaches correctly),
and refactor the patchFamilies local array into a private static readonly
string[] (e.g., PatchVisionFamilies) at class scope to avoid per-call
allocation, then update IsPatchVisionModel to reference that static field.

In `@src/CausalDiscovery/ContinuousOptimization/ContinuousOptimizationBase.cs`:
- Around line 66-110: The LossType check in ApplyOptions is case- and
whitespace-sensitive; normalize options.LossType (e.g., call Trim() and
ToLowerInvariant()) before validating and assigning to LossType so inputs like
"L2" or " l2 " are accepted; update the validation to use the normalized value
and assign that normalized string to LossType (refer to ApplyOptions and the
LossType/LossType validation logic).

In `@src/ComputerVision/Detection/Backbones/BackboneBase.cs`:
- Around line 99-101: BackboneBase is hiding the inherited GetParameterCount by
using "new abstract", breaking polymorphism; change the declaration in
BackboneBase (GetParameterCount) to override the inherited contract (e.g.,
replace "new abstract" with an override abstract declaration) so calls through
NeuralNetworkBase<T> or INeuralNetworkModel<T> dispatch to the backbone
implementation; also confirm the base class (NeuralNetworkBase<T> or its
ancestor) declares GetParameterCount as virtual/abstract and adjust signatures
to match (same return/type and accessibility) so the override compiles.

In `@src/ComputerVision/Detection/Necks/BiFPN.cs`:
- Around line 541-556: The DeepCopy implementation for BiFPN<T> uses
WriteParameters/ReadParameters which serializes weights via
NumOps.ToDouble/FromDouble and loses precision for non-double T; instead, after
creating clone = new BiFPN<T>((int[])_inputChannels.Clone(), _outputChannels,
_numRepeats) directly copy each tensor/parameter field from this instance into
clone (e.g., iterate over weight/bias tensors, attention/conv buffers, or any
parameter collections) and perform element-wise tensor copies or Clone()
operations that preserve the generic type T rather than piping through binary
serialization; remove the MemoryStream/Writer/Reader path and ensure every
unique parameter container in BiFPN<T> is copied by reference-preserving tensor
copy so the clone is an exact in-memory replica.

In `@src/ComputerVision/Detection/Necks/FPN.cs`:
- Around line 315-330: DeepCopy currently serializes parameters via
WriteParameters/ReadParameters which round-trips numeric values through
NumOps.ToDouble/FromDouble and loses precision for non-double T; change
FPN<T>.DeepCopy to create the clone (new FPN<T>((int[])_inputChannels.Clone(),
_outputChannels)) and then directly duplicate the model parameters/tensors
instead of calling WriteParameters/ReadParameters — iterate the internal
parameter collection used by FPN (the fields/properties that store
weights/biases), and for each Tensor<T> allocate a new tensor of the same shape
and copy contents element-wise or via the tensor's native Clone/Copy API (using
NumOps or Tensor.Copy methods that preserve T) before assigning them to the
clone, so no conversion to double occurs.

In `@src/ComputerVision/Detection/Necks/NeckBase.cs`:
- Around line 305-315: The Predict(Tensor<T>) method incorrectly forwards a
single Tensor to Forward(...) which expects the full backbone feature pyramid;
update the API so Predict is explicitly unsupported and cannot be used for
necks: mark NeckBase.Predict(Tensor<T>) to throw NotSupportedException with a
clear message directing callers to use a new public method that accepts
List<Tensor<T>> (e.g., Predict(List<Tensor<T>> inputFeatures) or reuse Forward
as the public inference entry), and update implementations (FPN<T>, PANet<T>,
BiFPN<T>) to implement the list-based entry; ensure the error message references
the class name via GetType().Name and explains that necks require multiple
feature maps.

In `@src/ComputerVision/Detection/Necks/PANet.cs`:
- Around line 404-419: DeepCopy currently recreates parameters via a
serialization round-trip using WriteParameters/ReadParameters which converts
tensors through doubles; instead instantiate the clone PANet<T> and copy each
parameter tensor directly (e.g. iterate the layer/parameter list or the
Parameters collection on the source, call the tensor-level clone/copy method
provided by the tensor type such as a Tensor<T>.Clone() or backend-specific
CloneTensor and assign those cloned tensors to the corresponding fields of the
new PANet<T>), avoiding NumOps.ToDouble/FromDouble and any binary writer/reader
path so the numeric type T is preserved exactly; replace the
MemoryStream/WriteParameters/ReadParameters logic in DeepCopy with this direct
per-tensor cloning and assignment while preserving the same
_inputChannels/_outputChannels initialization.

In `@src/Diffusion/Attention/MotionModule.cs`:
- Around line 105-115: The MotionModule<T> resolves its sublayers using layout
[H*W, numFrames, channels] in the Forward prep (calls like
_norm1.ResolveFromShape, _norm2.ResolveFromShape, _ffnIn.ResolveFromShape,
_ffnOut.ResolveFromShape) but the module's declared LayerBase<T> input/output
shape (the shape set at the class wrapper around lines where MotionModule<T>
sets its LayerBase input/output of [1, numFrames * H * W, channels]) uses [1,
numFrames*H*W,channels], causing inconsistent resolved-shape caches; pick one
canonical layout and make both match — either change the LayerBase<T>
input/output shape declaration to [H*W, numFrames, channels] (preferred if that
is the real contract) or change the ResolveFromShape calls to use [1,
numFrames*H*W, channels], and update any comments to reflect the chosen contract
so RequireResolved and ONNX export use the same topology.

In `@src/Diffusion/Conditioning/CLIPTextConditioner.cs`:
- Around line 153-179: The current masking only zeros input embeddings via
maskedOut (computed from attentionMask) before the transformer, but LayerNorm
and subsequent projections (e.g., any methods that compute sequence embeddings
or call FindEosPosition) can revive padded tokens; update the code to re-apply
the mask after the transformer/projection stage so masked positions remain zero
in the exported sequence embeddings or alternatively carry the attention
mask/EOS index through to FindEosPosition; locate usages of
maskedOut/attentionMask and the arrays hidden, TokenEmbeddings,
PositionEmbeddings, and ensure you zero the corresponding positions (or skip
them in projection outputs and LayerNorm results) right after the
projection/LayerNorm step(s) that produce inputs to FindEosPosition (also apply
the same fix where similar masking appears around lines 189-196 and 202-255).
- Around line 124-135: The attentionMask validation currently allows shapes with
more than two dimensions; update the check in CLIPTextConditioner.cs (the block
referencing attentionMask, maskShape, batchSize, seqLen, and shape) to require
an exact rank of 2 (maskShape.Length == 2) and verify maskShape[0] == batchSize
and maskShape[1] == seqLen; if the check fails, throw the ArgumentException with
the same descriptive message using the actual maskShape and
nameof(attentionMask) so callers get the precise error when a non-2D mask (e.g.,
[batchSize, seqLen, 1]) is passed.
- Around line 258-270: GetUnconditionalEmbedding currently builds input token
ids [BOS, EOS, PAD,...] but calls EncodeText(input) with a null attention mask
so PAD tokens are treated as active; change it to build an attentionMask tensor
of shape [batchSize, MaxSequenceLength] with values 1 for the first two
positions (BOS and EOS) and 0 for the rest for each batch row, and pass that
mask into EncodeText (use the existing EncodeText overload that accepts
attentionMask), ensuring types match the expected mask tensor type and
dimensions (refer to GetUnconditionalEmbedding, MaxSequenceLength, EncodeText).

In `@src/Diffusion/Control/IPAdapterModel.cs`:
- Around line 693-730: In FlattenPatches, add upfront validation to reject any
unsupported image shapes instead of proceeding with unsafe indexing: verify
image.Shape.Length == 4, image.Shape[0] == 1 (no batching), image.Shape[1] == 3
(expected channels), and image.Shape[2] == _imageSize and image.Shape[3] ==
_imageSize; if any check fails throw an ArgumentException (or
ArgumentOutOfRangeException) with a clear message referencing expected shape
"[1,3,_imageSize,_imageSize]" and the offending shape; keep the rest of the
method (variables _patchSize, _numPatches, hStride, cStride, and the flatten
loop) unchanged so behavior remains identical for supported inputs.

In `@src/Diffusion/NoisePredictors/NoisePredictorBase.cs`:
- Around line 690-709: ComputeGradientsWithTape currently hardcodes MSE loss;
change it to accept a tensor-space loss callback or ILossFunction<T> (e.g., add
a parameter Func<Tensor<T>,Tensor<T>,Tensor<T>> lossFn or ILossFunction<T> loss)
and use that to produce the scalar loss tensor from predicted and target instead
of building squared error inside the method; then call
tape.ComputeGradients(lossTensor, trainableParams) as before and update callers
(including ComputeGradients) to pass the configured loss implementation or
rename the method to make MSE-only explicit if you prefer the current behavior.
- Around line 79-84: The ReflectInstanceLayers default (used by
NoisePredictorBase and referenced in ReflectInstanceLayers and
EnumerateLayers()) currently does not recurse into owned reference-type objects
(e.g., List<Block> or wrapper objects), causing inner ILayer<T> instances to be
missed; update ReflectInstanceLayers to recursively walk reference-type fields
and properties (including IEnumerable/IDictionary elements) using the existing
cycle-guard mechanism so nested block graphs are discovered, or alternatively
make EnumerateLayers() abstract/enforced so implementers must explicitly
enumerate ownership; modify the implementation referenced in NoisePredictorBase
to use the recursive traversal with cycle detection to capture nested blocks.
- Around line 310-316: The LazyMHA factory currently computes per-head width
with integer division which silently truncates; update LazyMHA to validate that
embeddingDimension is divisible by headCount (e.g., embeddingDimension %
headCount == 0) and if not throw a clear ArgumentException (or
ArgumentOutOfRangeException) naming the parameters; then pass embeddingDimension
/ headCount into the MultiHeadAttentionLayer<T> constructor as before. Ensure
the exception message references LazyMHA, embeddingDimension and headCount so
misconfiguration fails fast.

In `@src/Diffusion/VAE/VAEModelBase.cs`:
- Around line 497-501: The current broad catch in VAEModelBase (the VAE layer
backpropagation block that logs "VAE layer backpropagation failed, falling back
to SPSA") masks real implementation errors; change the catch to only handle
expected capability errors (e.g., catch NotSupportedException,
NotImplementedException and/or a custom CapabilityNotAvailableException if you
have one) and keep the Trace.TraceWarning including full exception details, but
rethrow any other exceptions (or remove the catch) so shape bugs and other
unexpected failures surface; update the catch clauses around the
backpropagation/fallback-to-SPSA block accordingly and ensure you still log the
exception message and stack for the expected exceptions.
- Around line 555-574: The method ComputeGradientsWithTape currently exposes
low-level training plumbing publicly; change its accessibility to protected (or
internal) so library consumers can't call it directly. Locate
ComputeGradientsWithTape in VAEModelBase and update its modifier from public to
protected (or internal) while keeping the implementation (uses GradientTape,
ForwardForTraining, Engine.* operations, and tape.ComputeGradients) unchanged;
ensure any internal callers still compile and update unit/tests if they relied
on the public surface.

In `@src/Interpretability/Explainers/InfluenceFunctionExplainer.cs`:
- Around line 450-473: The ComputeNumericalGradients method and the
numerical-gradient fallback branch in ComputeGradient are dead/duplicate code
because constructor invariants guarantee either _network or _gradientFunction is
set; remove the unused method ComputeNumericalGradients and delete the
unreachable fallback branch inside ComputeGradient (the branch that uses
numerical approximation) to eliminate dead code and duplicate paths, and ensure
any calls or references to ComputeNumericalGradients are removed or redirected
to the existing _gradientFunction/_network gradient logic so only the validated
gradient code paths remain.

In `@src/LoRA/Adapters/AdaLoRAAdapter.cs`:
- Around line 296-297: The rank bookkeeping bug comes from compacting scores
into [0.._currentRank) while leaving matrix components at their original
indices, then updating gradients using for (int r = 0; r < _currentRank; r++)
which mismatches active components; fix by introducing and maintaining an
activeIndices array/list (e.g., List<int> activeIndices) that maps the compacted
score slot to the original component index whenever you prune/compact (update
the compaction block that currently moves scores into [0.._currentRank) to also
populate activeIndices and move or remap the corresponding matrix components),
then replace all loops that iterate with r < _currentRank (including the
gradient update loop in AdaLoRAAdapter and the other occurrences around the
locations mentioned) to iterate over activeIndices (for each origIdx in
activeIndices) and use origIdx to read/write gradients, scores, and matrix
component storage so pruning stays consistent and future importance updates
reference the correct components.
- Around line 560-563: The current placeholder that delegates merge behavior
must be replaced with an explicit active-rank merge: remove the "for now"
delegation and implement a BuildMergedActiveRankWeights method that constructs
the merged Matrix<T> by iterating only over active rank indices and combining
the corresponding _loraLayer weight matrices (and any bias/scale terms) into the
result without relying on prior zeroing side effects; update the merge call
sites in AdaLoRAAdapter (the existing merge code around the LoRA layer) to call
BuildMergedActiveRankWeights and ensure the merging respects the same indexing
convention as the _loraLayer matrices and preserves data types and shapes.

---

Duplicate comments:
In `@src/ComputerVision/Detection/Backbones/ConvUtils.cs`:
- Around line 54-59: The Conv2D constructor currently forces a bias term,
breaking the prior useBias=false contract and altering params/serialization;
update Conv2D (the Conv2D(int inChannels, int outChannels, int kernelSize, int
stride = 1, int padding = 0) constructor) to accept a bool useBias parameter and
propagate it into the underlying ConvolutionalLayer<T> (or, if
ConvolutionalLayer<T> lacks bias support, add a useBias flag to
ConvolutionalLayer<T> and its ctor and serialization logic), ensure call sites
creating new Conv2D<> are updated or overloaded to preserve backward
compatibility, and adjust serialization/versioning in the ConvolutionalLayer<T>
(and any Serialize/Deserialize methods) to maintain checkpoint compatibility
when useBias changes.

In `@src/Diffusion/Conditioning/CLIPTextConditioner.cs`:
- Around line 298-335: The loop inside ApplyTransformerLayers currently uses
LinearProject for both attention and MLP; replace those surrogates with the real
CLIP transformer block flow: call the existing attention primitive (the
production self-attention method) with the correct slice of TransformerWeights,
headDim/NumHeads and pass the attentionMask through, apply residual + LayerNorm
semantics exactly as in the real block (use CopyVector/CopyVector for residuals,
LayerNorm for pre/post norms), then call the production feed-forward (MLP)
primitive instead of LinearProject using the correct MLP weight offsets from
TransformerWeights; ensure you use the same lnGamma/lnBeta extraction already
present, preserve residual connection ordering (residual -> LayerNorm ->
Attention -> add residual -> LayerNorm -> MLP -> add residual), and remove the
two LinearProject calls so ApplyTransformerLayers, LinearProject,
TransformerWeights, LayerNorm, AddVectors, CopyVector and attentionMask are
correctly wired to the production attention + feed-forward primitives.

In `@src/Diffusion/NoisePredictors/NoisePredictorBase.cs`:
- Around line 657-669: The base method ComputeGradients in NoisePredictorBase
currently throws NotSupportedException causing runtime failures when Train calls
it; either make ComputeGradients abstract to force concrete predictors to
implement it, or implement a production-ready default that gathers the
predictor's trainable tensors and delegates to ComputeGradientsWithTape (mirror
the pattern in DiffusionModelBase.ComputeGradients): update
NoisePredictorBase.ComputeGradients to be abstract or to call the predictor's
collected trainable tensors and invoke ComputeGradientsWithTape(input, target,
trainables, lossFunction) so Train no longer depends on a runtime stub.

In `@src/Diffusion/VAE/VAEDecoder.cs`:
- Around line 199-258: The lazy decoder convolutions (_postQuantConv,
_inputConv, _outputConv) are instantiated but never ResolveFromShape(...)'d,
leaving parameter lists incomplete until Forward() runs; to fix, call
ResolveFromShape(...) on each of those layers in the VAEDecoder<T> constructor
using the correct tensor shapes (e.g. use latentChannels/_bottleneckSize for
_postQuantConv, lastChannels and _bottleneckSize for _inputConv, and
outputChannels with final spatial size for _outputConv) so their parameters are
initialized immediately and GetParameters()/SetParameters()/checkpointing work
without needing Forward().

In `@src/Diffusion/VAE/VAEModelBase.cs`:
- Around line 576-592: The BackpropagateLossGradient(Tensor<T> lossGradient)
method in VAEModelBase is a no-op placeholder causing silent fallback to SPSA;
change its signature to be abstract (declare protected abstract void
BackpropagateLossGradient(Tensor<T> lossGradient)) so concrete VAE
implementations must provide an exact-gradient path, and update all subclasses
of VAEModelBase to implement BackpropagateLossGradient (calling their
layer.Backward(grad) or tape-based backward logic) to restore the primary
exact-gradient flow; ensure the base class is made abstract if needed and adjust
any callers/tests to account for the now-required implementation.

🪄 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: fab48353-6ae8-4cd1-b9cf-6c6fc8294fd6

📥 Commits

Reviewing files that changed from the base of the PR and between 96cd76a and 2973775.

📒 Files selected for processing (34)

• src/AiDotNet.Generators/TestScaffoldGenerator.cs
• src/Audio/Enhancement/DCCRN.cs
• src/Autodiff/NeuralNetworkDerivatives.cs
• src/CausalDiscovery/ContinuousOptimization/ContinuousOptimizationBase.cs
• src/ComputerVision/Detection/Backbones/BackboneBase.cs
• src/ComputerVision/Detection/Backbones/CSPDarknet.cs
• src/ComputerVision/Detection/Backbones/ConvUtils.cs
• src/ComputerVision/Detection/Backbones/EfficientNet.cs
• src/ComputerVision/Detection/Backbones/ResNet.cs
• src/ComputerVision/Detection/Backbones/SwinTransformer.cs
• src/ComputerVision/Detection/Necks/BiFPN.cs
• src/ComputerVision/Detection/Necks/FPN.cs
• src/ComputerVision/Detection/Necks/NeckBase.cs
• src/ComputerVision/Detection/Necks/PANet.cs
• src/ComputerVision/Detection/ObjectDetection/YOLO/YOLOHead.cs
• src/ComputerVision/Detection/ObjectDetection/YOLO/YOLOv9.cs
• src/Diffusion/Attention/MotionModule.cs
• src/Diffusion/Attention/STDiTBlock.cs
• src/Diffusion/Attention/TemporalSelfAttention.cs
• src/Diffusion/Conditioning/CLIPTextConditioner.cs
• src/Diffusion/Control/IPAdapterFaceIDPlusModel.cs
• src/Diffusion/Control/IPAdapterModel.cs
• src/Diffusion/Control/IPAdapterPlusModel.cs
• src/Diffusion/NoisePredictors/NoisePredictorBase.cs
• src/Diffusion/ThreeD/DreamFusionModel.cs
• src/Diffusion/VAE/Causal3DVAE.cs
• src/Diffusion/VAE/LiteVAEModel.cs
• src/Diffusion/VAE/TemporalInterpolationVAE.cs
• src/Diffusion/VAE/VAEDecoder.cs
• src/Diffusion/VAE/VAEEncoder.cs
• src/Diffusion/VAE/VAEModelBase.cs
• src/Interpretability/Explainers/InfluenceFunctionExplainer.cs
• src/LoRA/Adapters/AdaLoRAAdapter.cs
• src/NeuralNetworks/Layers/LayerBase.cs

[coderabbitai]

Actionable comments posted: 18

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

src/Diffusion/Attention/MotionModule.cs (1)

43-43: ⚠️ Potential issue | 🟡 Minor

_lastInput is stored but never read — likely dead code.

Line 43 declares _lastInput and line 123 assigns to it, but the field is never subsequently accessed. This is typically used for caching input during the backward pass, yet no Backward method exists. Either this is dead code that should be removed, or there's an incomplete implementation for gradient computation.

♻️ If not needed, remove the dead field

-    private Tensor<T>? _lastInput;

And in Forward:

     public override Tensor<T> Forward(Tensor<T> input)
     {
-        _lastInput = input;
-
         // Temporal attention with residual

Also update ResetState:

     public override void ResetState()
     {
-        _lastInput = null;
         _temporalAttention.ResetState();

Also applies to: 123-123

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Diffusion/Attention/MotionModule.cs` at line 43, The private field
_lastInput is assigned in Forward but never read, indicating dead state
handling; either remove _lastInput and its assignments (clean up in Forward and
ResetState) or implement a Backward(Tensor<T> grad) method that uses _lastInput
for gradient computation and keep ResetState to clear it. Locate the _lastInput
declaration and the assignment in Forward, then either delete the field and
related assignments/clears (including in ResetState) or add a Backward method
that consumes _lastInput and document/clear it in ResetState to avoid lingering
state.

src/Diffusion/Control/IPAdapterModel.cs (1)

600-601: 🧹 Nitpick | 🔵 Trivial

Consider access modifier scope for helper classes.

ImageEncoder<T> and ImageProjector<T> are currently public. If they're only consumed internally by IPAdapterModel<T> and not intended as extension points, making them internal would reduce the public API surface per the facade pattern guidelines. However, if users should be able to instantiate or extend these independently (e.g., for custom IP-Adapter variants), keeping them public is appropriate.

Also applies to: 844-845

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Diffusion/Control/IPAdapterModel.cs` around lines 600 - 601,
ImageEncoder<T> and ImageProjector<T> are declared public but appear to be
helper types for IPAdapterModel<T>; if they are not part of the intended public
API, change their declarations from public to internal to reduce surface area
(update both ImageEncoder<T> and ImageProjector<T>), otherwise leave them public
if external instantiation/extension is required; search usages of
ImageEncoder<T>, ImageProjector<T> and IPAdapterModel<T> to confirm no external
references before making the change.

src/ComputerVision/Detection/Backbones/CSPDarknet.cs (1)

83-89: ⚠️ Potential issue | 🔴 Critical

Restore explicit useBias: false to Conv2D calls or confirm checkpoint/test compatibility.

Evidence indicates AiDotNet defaults bias to true (matching PyTorch, Keras, and visible in DenseLayer/ConvolutionalLayer implementations). Removing useBias: false silently changes the backbone's parameter layout and checkpoint compatibility. This is a BLOCKING issue.

Required before merge:

1. Confirm Conv2D constructor defaults useBias to true by inspecting src/ComputerVision/Detection/Layers/Conv2D.cs
2. If confirmed, either:
   • Restore useBias: false to all Conv2D calls in this PR (lines 83–89, 228–243, 246–252, 262–268, 379–393 in CSPDarknet.cs, plus sibling backbones ResNet.cs, EfficientNet.cs)
   • OR provide migration path: update serialized checkpoint loader, add regression tests asserting parameter shapes/counts match old checkpoints, and document the breaking change

Also applies to: lines 228–243, 246–252, 262–268, 379–393 in CSPDarknet.cs, and matching changes in sibling backbone files.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ComputerVision/Detection/Backbones/CSPDarknet.cs` around lines 83 - 89,
The Conv2D<T> calls in CSPDarknet (e.g., the _stem field and other Conv2D<T>
instantiations) were changed such that they now rely on the constructor default
for bias; inspect Conv2D.cs to confirm its default is useBias: true, and if so
revert all Conv2D<T> calls in CSPDarknet.cs (including the _stem and the other
Conv2D<T> usages referenced in the review) to explicitly pass useBias: false to
restore the original parameter layout and checkpoint compatibility;
alternatively, if you intend to change the bias default, implement a migration
by updating the checkpoint loader to map the new parameter shapes, add
regression tests that assert parameter counts/shapes match existing checkpoints,
and document the breaking change—do one of these two fixes before merging.

♻️ Duplicate comments (4)

src/Diffusion/NoisePredictors/NoisePredictorBase.cs (2)

727-740: ⚠️ Potential issue | 🔴 Critical

Blocking: base ComputeGradients() still ships as a runtime stub.

Train() calls this method, but the base implementation only throws. Any predictor that misses the override will compile cleanly and fail on the first training step. Make it abstract, or add an abstract trainable-tensor enumeration so the base can implement the tape path for real.

Proposed fix

-    public virtual Vector<T> ComputeGradients(Tensor<T> input, Tensor<T> target, ILossFunction<T>? lossFunction = null)
-    {
-        if (input == null)
-            throw new ArgumentNullException(nameof(input));
-        if (target == null)
-            throw new ArgumentNullException(nameof(target));
-
-        throw new NotSupportedException(
-            $"{GetType().Name} does not implement ComputeGradients. " +
-            "Override this method on the concrete predictor and route through " +
-            "ComputeGradientsWithTape with the predictor's collected trainable tensors. " +
-            "AiDotNet has no per-layer Backward; autodiff goes through the GradientTape " +
-            "(see DiffusionModelBase.ComputeGradients for the canonical pattern).");
-    }
+    public abstract Vector<T> ComputeGradients(
+        Tensor<T> input,
+        Tensor<T> target,
+        ILossFunction<T>? lossFunction = null);

As per coding guidelines "Every PR must contain production-ready code" and "All methods have complete, production-ready implementations."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Diffusion/NoisePredictors/NoisePredictorBase.cs` around lines 727 - 740,
The base ComputeGradients(Tensor<T> input, Tensor<T> target, ILossFunction<T>?
lossFunction = null) is a runtime stub that currently throws; make it
production-ready by either declaring it abstract or implementing the tape-based
path: add an abstract member (e.g., IEnumerable<Tensor<T>> GetTrainableTensors()
or a TrainableTensors property) that concrete predictors must implement, then
implement ComputeGradients to validate inputs and call ComputeGradientsWithTape
using GetTrainableTensors()/TrainableTensors so Train() can safely call
ComputeGradients without runtime failures (see ComputeGradientsWithTape and
DiffusionModelBase.ComputeGradients for the canonical pattern).

---

767-794: 🛠️ Refactor suggestion | 🟠 Major

Narrow ComputeGradientsWithTape() out of the public surface.

This is low-level autodiff plumbing on a base class, not a facade entry point. Leaving it public turns GradientTape plus raw trainable-tensor ownership into supported API.

Proposed fix

-    public Dictionary<Tensor<T>, Tensor<T>> ComputeGradientsWithTape(
+    protected Dictionary<Tensor<T>, Tensor<T>> ComputeGradientsWithTape(
         Tensor<T> input,
         Tensor<T> target,
         Tensor<T>[] trainableParams,
         Func<Tensor<T>, Tensor<T>, Tensor<T>>? lossBuilder = null)

As per coding guidelines "Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs" and "Prefer internal over public for plumbing/helper classes that users never instantiate or consume".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Diffusion/NoisePredictors/NoisePredictorBase.cs` around lines 767 - 794,
The ComputeGradientsWithTape method on NoisePredictorBase is public but is
internal autodiff plumbing; change its declaration from public to internal
(i.e., internal Dictionary<Tensor<T>, Tensor<T>> ComputeGradientsWithTape(...))
so GradientTape and raw trainable-tensor handling are not exposed on the public
API; ensure any callers are internal to the same assembly (or add
InternalsVisibleTo for test assemblies if needed) and leave the method body and
referenced symbols (GradientTape<T>, Forward, Engine, tape.ComputeGradients)
unchanged.

src/Diffusion/VAE/VAEModelBase.cs (1)

558-591: ⚠️ Potential issue | 🟠 Major

Pass the selected loss into ComputeGradientsWithTape().

ComputeGradients(...) accepts an arbitrary ILossFunction<T>, but this helper always backpropagates MSE. Any subclass that routes training through this helper with MAE/KL/etc. will silently optimize the wrong objective.

Proposed fix

-    protected Dictionary<Tensor<T>, Tensor<T>> ComputeGradientsWithTape(
-        Tensor<T> input,
-        Tensor<T> target,
-        Tensor<T>[] trainableParams)
+    protected Dictionary<Tensor<T>, Tensor<T>> ComputeGradientsWithTape(
+        Tensor<T> input,
+        Tensor<T> target,
+        Tensor<T>[] trainableParams,
+        Func<Tensor<T>, Tensor<T>, Tensor<T>>? lossBuilder = null)
     {
         using var tape = new GradientTape<T>();
 
         // Forward pass (recorded by the engine)
         var predicted = ForwardForTraining(input);
 
-        // Compute MSE loss using tape-recorded engine ops
-        var diff = Engine.TensorSubtract(predicted, target);
-        var squared = Engine.TensorMultiply(diff, diff);
-        // ReduceMean with all axes produces a scalar tensor that the tape can differentiate
-        var allAxes = Enumerable.Range(0, squared.Shape.Length).ToArray();
-        var loss = Engine.ReduceMean(squared, allAxes, keepDims: false);
+        Tensor<T> loss;
+        if (lossBuilder is not null)
+        {
+            loss = lossBuilder(predicted, target);
+        }
+        else
+        {
+            var diff = Engine.TensorSubtract(predicted, target);
+            var squared = Engine.TensorMultiply(diff, diff);
+            var allAxes = Enumerable.Range(0, squared.Shape.Length).ToArray();
+            loss = Engine.ReduceMean(squared, allAxes, keepDims: false);
+        }
 
         // Reverse-mode AD: compute gradients for all trainable parameters
         return tape.ComputeGradients(loss, trainableParams);
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Diffusion/VAE/VAEModelBase.cs` around lines 558 - 591,
ComputeGradientsWithTape currently always computes MSE internally, causing
callers that pass a different ILossFunction<T> (via ComputeGradients) to
backprop the wrong objective; change the method signature of
ComputeGradientsWithTape (and its callers such as ComputeGradients) to accept
either the selected ILossFunction<T> lossFunc or a precomputed loss Tensor<T>,
then compute the loss via lossFunc (e.g., lossFunc.ComputeLoss(predicted,
target)) or use the supplied loss Tensor instead of the hard-coded MSE code
(replace Engine.TensorSubtract/Multiply/ReduceMean with the delegated loss
computation); update references to ForwardForTraining and any call sites to pass
the chosen loss/lossFunc through.

src/ComputerVision/Detection/Backbones/BackboneBase.cs (1)

171-182: ⚠️ Potential issue | 🔴 Critical

Owned backbone layers are still invisible to base disposal.

Documenting InitializeLayers() as a no-op leaves _stem, stages, MBConv blocks, and other owned wrappers outside the inherited layer graph. Any disposal path that walks NeuralNetworkBase<T> children will miss them, so lazily allocated tensors never get returned to the allocator. Register owned layers here, or add an explicit ownership/disposal hook that derived backbones must implement.

🔧 One way to make ownership explicit

-protected override void InitializeLayers()
-{
-    // Intentional no-op — see XML doc above.
-}
+protected override void InitializeLayers()
+{
+    RegisterOwnedLayers();
+}
+
+protected abstract void RegisterOwnedLayers();

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ComputerVision/Detection/Backbones/BackboneBase.cs` around lines 171 -
182, The InitializeLayers override currently left as a no-op causes owned
members (e.g., _stem, stage wrappers, MBConv blocks) to be invisible to
NeuralNetworkBase<T> disposal; update InitializeLayers to register all owned
layer wrappers with the inherited Layers collection (e.g., call
Layers.Add(_stem), add each stage/MBConv wrapper and any nested sub-layers) so
they participate in base traversal, or alternatively introduce a protected
virtual DisposeOwnedLayers/ReleaseOwnedLayers method on BackboneBase that
derived backbones implement and call from the base Dispose path; ensure
ExtractFeatures, WriteParameters, and ReadParameters semantics remain unchanged
while making ownership explicit so lazy tensors are released.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/ComputerVision/Detection/Backbones/BackboneBase.cs`:
- Around line 197-198: CreateNewInstance currently returns a shallow clone via
MemberwiseClone, which reuses _stem, stage lists and nested tensors; replace
this by making CreateNewInstance abstract on BackboneBase<T> (remove
MemberwiseClone usage) and require each concrete backbone to implement
CreateNewInstance to construct and return a brand-new instance (new constructor
call) with freshly allocated _stem, stage collections and layers/tensors so no
shared mutable state is reused across instances.

In `@src/ComputerVision/Detection/Necks/FPN.cs`:
- Around line 332-343: CopyTensorInto currently only compares src.Length and
dst.Length which allows different shapes with same total elements; update
CopyTensorInto to validate full tensor shape by comparing Rank and each
dimension (e.g., src.Shape or src.Dimensions) against dst's corresponding shape
before copying, and throw the InvalidOperationException including both full
shapes when they differ; keep the element-wise copy loop unchanged once shapes
are confirmed equal.

In `@src/ComputerVision/Detection/Necks/NeckBase.cs`:
- Around line 170-206: Downsample2x currently uses floor division and unguarded
accesses which drop odd rows/cols and risk out-of-range reads; change it to
compute outHeight = (height + 1) / 2 and outWidth = (width + 1) / 2, then for
each output cell in Downsample2x collect only the valid source neighbors (check
that h*2, h*2+1 < height and w*2, w*2+1 < width) before computing the max; use
Tensor<T> indexing and NumOps.GreaterThan as before but guard each val access
and ignore or skip invalid neighbors so odd spatial dimensions are preserved and
pyramid alignment remains correct.

In `@src/ComputerVision/Detection/Necks/PANet.cs`:
- Around line 430-441: CopyTensorInto currently only compares Length, allowing
tensors with different shapes to slip through; update the method (CopyTensorInto
in class PANet where Tensor<T> is used) to validate that src.Rank == dst.Rank
and that every dimension size matches (for each axis confirm src.GetDimension(i)
== dst.GetDimension(i) or use src.Dimensions[i]/dst.Dimensions[i] depending on
the Tensor API), and if any mismatch throw an InvalidOperationException
containing both tensor shapes (rank and per-dimension sizes); after these checks
keep the existing element-wise copy loop unchanged.

In `@src/Diffusion/Attention/MotionModule.cs`:
- Around line 99-100: The explicit casts to IActivationFunction<T> are redundant
when constructing DenseLayer<T>; update the _ffnIn and _ffnOut instantiations to
pass new GELUActivation<T>() and new IdentityActivation<T>() directly (remove
(IActivationFunction<T>) casts) so DenseLayer<T> receives the activation
instances without unnecessary casting; adjust the lines that assign _ffnIn and
_ffnOut accordingly.

In `@src/Diffusion/Conditioning/CLIPTextConditioner.cs`:
- Line 336: The variable headDim (computed as "int headDim = HiddenSize /
NumHeads;") is dead code—remove the unused calculation from
CLIPTextConditioner.cs to avoid confusion; if the intent was to validate or use
the per-head dimension, instead add a validation using HiddenSize and NumHeads
(e.g., assert HiddenSize % NumHeads == 0) or replace the removed line with a
clear use of headDim where needed, referencing the same symbols HiddenSize and
NumHeads.
- Around line 282-304: In GetUnconditionalEmbedding replace the incorrect BOS
token assignment (currently setting tokenIds[...] via NumOps.FromDouble(1)) with
the standard CLIP BOS id by using VocabSize - 2 (i.e. tokenIds[b *
MaxSequenceLength] = NumOps.FromDouble(VocabSize - 2)); ensure this change is
made where tokenIds is initialized so BOS matches EOS (VocabSize - 1) and the
vocabulary convention used across EncodeText; if BOS=1 was intentional instead,
add class-level documentation describing the custom token-id scheme and why it
differs from standard CLIP.

In `@src/Diffusion/NoisePredictors/NoisePredictorBase.cs`:
- Around line 466-471: The Predict method in NoisePredictorBase currently
hardcodes a timestep (calls PredictNoise(input, 500, null)), which is invalid
for timestep-conditional models; change Predict to fail-fast instead of using
the magic number: either (A) make Predict abstract/virtual with no default
implementation so concrete predictors must implement a timestep-aware Predict
overload, or (B) have Predict throw an
InvalidOperationException/NotSupportedException with a clear message instructing
callers to call PredictNoise(input, timestep, context) or use the timestep-aware
API; update any callers to pass the correct timestep and context and remove
reliance on NoGradScope<T> only if still appropriate. Ensure references to
Predict and PredictNoise are updated so no code path silently uses the hardcoded
500.
- Around line 92-93: The base implementation of EnumerateLayers currently
returns ReflectInstanceLayers(this), causing shared/injected layers to be
considered owned and eligible for disposal (DisposeOnceGuard won't prevent
tearing down shared dependencies); change the base method EnumerateLayers to
return an empty enumeration (e.g., Enumerable.Empty<ILayer<T>>()) so
reflection-based disposal is opt-in, and update concrete predictor classes that
truly own layers to override EnumerateLayers and call
ReflectInstanceLayers(this) as needed; ensure callers that rely on owned-layer
enumeration (and any use of DisposeOnceGuard) remain correct after this change.

In `@src/Diffusion/VAE/VAEModelBase.cs`:
- Around line 515-543: The SPSA fallback mutates model parameters but doesn't
guarantee restoration on exceptions; wrap the parameter perturbation and
loss-evaluation loop so that original parameters (obtained from GetParameters())
are always restored in a finally block. Specifically, capture parameters once
via GetParameters(), perform the inner perturbation loop that calls
SetParameters(...), Predict(...), and effectiveLossFunction.CalculateLoss(...),
and ensure SetParameters(parameters) is invoked in a finally clause (not just
after the loop) so gradients_spsa is computed safely even if SetParameters,
Predict, or CalculateLoss throws; refer to GetParameters, SetParameters,
Predict, effectiveLossFunction.CalculateLoss, and gradients_spsa to locate the
code to change.
- Around line 606-609: The default backprop hook
BackpropagateLossGradient(Tensor<T> lossGradient) in VAEModelBase currently
no-ops which allows models that implement GetParameterGradients() to
accidentally fall through to stale/zero gradients; replace the empty body with
an explicit throw new NotSupportedException with a clear message (e.g.,
indicating that derived VAEs must override BackpropagateLossGradient to support
exact backprop and that SPSA fallback will be used otherwise) so unsupported
models hit an explicit failure path; ensure the method signature remains
protected virtual and update any concrete VAE implementations that do support
backprop to override BackpropagateLossGradient accordingly.

In `@src/Interpretability/Explainers/InfluenceFunctionExplainer.cs`:
- Around line 698-705: In InfluenceFunctionExplainer (inside the loop that calls
ComputeGradient for each training sample) validate that the returned grad length
matches the expected numParams before writing into _cachedTrainingGradients:
call ComputeGradient(...) and if grad.Length != numParams then throw/raise an
exception (or fail fast with a clear message) so inconsistent gradient widths
are detected immediately rather than silently truncating/zero-filling; do this
check for each sample (the loop that currently begins with for (int i = 1; i <
numSamples; i++)) to prevent corrupt downstream influence scores.
- Around line 587-615: The ComputeHessianVectorProduct method currently perturbs
input rather than model parameters and returns an invalid parameter-space HVP;
replace this with a fail-fast guard until a true parameter-space HVP is
implemented: detect that parameter access/HVP support is unavailable (i.e.,
where ComputeHessianVectorProduct is defined) and throw a clear
NotSupportedException or InvalidOperationException indicating "parameter-space
Hessian-vector product not implemented; influence functions require parameter
access", instead of performing input-based finite differences (remove/disable
the input perturbation loop that uses input.Length and ComputeGradient on
perturbed inputs); add a unit-testable path or flag (e.g., a boolean capability
check or method IsParameterSpaceHvpSupported) so callers can check support
before invoking ComputeHessianVectorProduct.
- Around line 417-428: The code in InfluenceFunctionExplainer that accumulates
into tracInScores currently truncates checkpointGrad columns to testGradient
length using Math.Min, which hides mismatched parameter widths; replace that
behavior with an explicit validation: check that checkpointGrad.Columns ==
testGradient.Length (and throw an ArgumentException with a clear message if not)
before the outer loop that iterates i from 0..numTrainingSamples-1 so you fail
fast on bad checkpoint matrices instead of silently computing wrong dot products
for tracInScores; locate the block referencing checkpointGrad, testGradient,
tracInScores and numTrainingSamples to implement this validation.

In `@src/LoRA/Adapters/AdaLoRAAdapter.cs`:
- Around line 414-423: Duplicate logic that packs matrixA and matrixB into the
LoRA parameter vector appears in PruneRank() and ExpandRank(); extract it into a
private helper (e.g., SyncMatricesToParameters(Matrix<T> matrixA, Matrix<T>
matrixB)) that reads the current parameter Vector<T> via
_loraLayer.GetParameters(), copies matrixA then matrixB values into the vector
preserving any trailing parameters, and calls
_loraLayer.SetParameters(loraParams); then replace the duplicated packing blocks
in PruneRank and ExpandRank with a single call to
SyncMatricesToParameters(matrixA, matrixB).
- Around line 486-495: The parameter packing is overwriting any existing bias or
extra params because you create a fresh zeroed Vector<T> loraParams; instead
retrieve the layer's existing parameters via _loraLayer.GetParameters() (size
_loraLayer.ParameterCount) and write matrixA and matrixB values into that vector
(using idx as now) before calling _loraLayer.SetParameters(loraParams) so other
parameters are preserved; update the code that builds loraParams (and references
to loraParams) to use GetParameters() instead of new Vector<T>(...).
- Around line 88-93: The documentation for the field _rankPruningThreshold is
misleading: it says "Components with importance scores below this threshold" but
the code (see usage in the expression that computes pruned count from
_currentRank, e.g. (int)(_currentRank * (1.0 - _rankPruningThreshold))) treats
the value as a fraction of ranks to keep/prune rather than a direct score
cutoff. Update the XML comment for _rankPruningThreshold to state it is a
fractional pruning parameter (e.g., fraction of ranks to prune or fraction to
retain) and explain how it is applied (used with _currentRank to compute the
number of ranks to keep/prune), and include valid range (0.0–1.0).
- Around line 297-300: The UpdateImportanceScores method calls
_loraLayer.GetParameterGradients() but does not guard against a null or
zero-length result, which can cause index/out-of-bounds during the gradient
magnitude loop; modify UpdateImportanceScores to check the returned Vector<T>
from _loraLayer.GetParameterGradients() for null and for Length == 0 (or
equivalent IsEmpty) and return early or skip the importance update when no
gradients are present, optionally emitting a diagnostic via the same logger used
in this class to aid debugging; keep the rest of the method unchanged so the
loop only runs when a valid non-empty gradient vector is available.

---

Outside diff comments:
In `@src/ComputerVision/Detection/Backbones/CSPDarknet.cs`:
- Around line 83-89: The Conv2D<T> calls in CSPDarknet (e.g., the _stem field
and other Conv2D<T> instantiations) were changed such that they now rely on the
constructor default for bias; inspect Conv2D.cs to confirm its default is
useBias: true, and if so revert all Conv2D<T> calls in CSPDarknet.cs (including
the _stem and the other Conv2D<T> usages referenced in the review) to explicitly
pass useBias: false to restore the original parameter layout and checkpoint
compatibility; alternatively, if you intend to change the bias default,
implement a migration by updating the checkpoint loader to map the new parameter
shapes, add regression tests that assert parameter counts/shapes match existing
checkpoints, and document the breaking change—do one of these two fixes before
merging.

In `@src/Diffusion/Attention/MotionModule.cs`:
- Line 43: The private field _lastInput is assigned in Forward but never read,
indicating dead state handling; either remove _lastInput and its assignments
(clean up in Forward and ResetState) or implement a Backward(Tensor<T> grad)
method that uses _lastInput for gradient computation and keep ResetState to
clear it. Locate the _lastInput declaration and the assignment in Forward, then
either delete the field and related assignments/clears (including in ResetState)
or add a Backward method that consumes _lastInput and document/clear it in
ResetState to avoid lingering state.

In `@src/Diffusion/Control/IPAdapterModel.cs`:
- Around line 600-601: ImageEncoder<T> and ImageProjector<T> are declared public
but appear to be helper types for IPAdapterModel<T>; if they are not part of the
intended public API, change their declarations from public to internal to reduce
surface area (update both ImageEncoder<T> and ImageProjector<T>), otherwise
leave them public if external instantiation/extension is required; search usages
of ImageEncoder<T>, ImageProjector<T> and IPAdapterModel<T> to confirm no
external references before making the change.

---

Duplicate comments:
In `@src/ComputerVision/Detection/Backbones/BackboneBase.cs`:
- Around line 171-182: The InitializeLayers override currently left as a no-op
causes owned members (e.g., _stem, stage wrappers, MBConv blocks) to be
invisible to NeuralNetworkBase<T> disposal; update InitializeLayers to register
all owned layer wrappers with the inherited Layers collection (e.g., call
Layers.Add(_stem), add each stage/MBConv wrapper and any nested sub-layers) so
they participate in base traversal, or alternatively introduce a protected
virtual DisposeOwnedLayers/ReleaseOwnedLayers method on BackboneBase that
derived backbones implement and call from the base Dispose path; ensure
ExtractFeatures, WriteParameters, and ReadParameters semantics remain unchanged
while making ownership explicit so lazy tensors are released.

In `@src/Diffusion/NoisePredictors/NoisePredictorBase.cs`:
- Around line 727-740: The base ComputeGradients(Tensor<T> input, Tensor<T>
target, ILossFunction<T>? lossFunction = null) is a runtime stub that currently
throws; make it production-ready by either declaring it abstract or implementing
the tape-based path: add an abstract member (e.g., IEnumerable<Tensor<T>>
GetTrainableTensors() or a TrainableTensors property) that concrete predictors
must implement, then implement ComputeGradients to validate inputs and call
ComputeGradientsWithTape using GetTrainableTensors()/TrainableTensors so Train()
can safely call ComputeGradients without runtime failures (see
ComputeGradientsWithTape and DiffusionModelBase.ComputeGradients for the
canonical pattern).
- Around line 767-794: The ComputeGradientsWithTape method on NoisePredictorBase
is public but is internal autodiff plumbing; change its declaration from public
to internal (i.e., internal Dictionary<Tensor<T>, Tensor<T>>
ComputeGradientsWithTape(...)) so GradientTape and raw trainable-tensor handling
are not exposed on the public API; ensure any callers are internal to the same
assembly (or add InternalsVisibleTo for test assemblies if needed) and leave the
method body and referenced symbols (GradientTape<T>, Forward, Engine,
tape.ComputeGradients) unchanged.

In `@src/Diffusion/VAE/VAEModelBase.cs`:
- Around line 558-591: ComputeGradientsWithTape currently always computes MSE
internally, causing callers that pass a different ILossFunction<T> (via
ComputeGradients) to backprop the wrong objective; change the method signature
of ComputeGradientsWithTape (and its callers such as ComputeGradients) to accept
either the selected ILossFunction<T> lossFunc or a precomputed loss Tensor<T>,
then compute the loss via lossFunc (e.g., lossFunc.ComputeLoss(predicted,
target)) or use the supplied loss Tensor instead of the hard-coded MSE code
(replace Engine.TensorSubtract/Multiply/ReduceMean with the delegated loss
computation); update references to ForwardForTraining and any call sites to pass
the chosen loss/lossFunc through.

🪄 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: d9646193-13e4-446d-929e-8237fadf5c62

📥 Commits

Reviewing files that changed from the base of the PR and between 2973775 and 06e3acf.

📒 Files selected for processing (18)

• src/AiDotNet.Generators/TestScaffoldGenerator.cs
• src/CausalDiscovery/ContinuousOptimization/ContinuousOptimizationBase.cs
• src/ComputerVision/Detection/Backbones/BackboneBase.cs
• src/ComputerVision/Detection/Backbones/CSPDarknet.cs
• src/ComputerVision/Detection/Backbones/EfficientNet.cs
• src/ComputerVision/Detection/Backbones/ResNet.cs
• src/ComputerVision/Detection/Backbones/SwinTransformer.cs
• src/ComputerVision/Detection/Necks/BiFPN.cs
• src/ComputerVision/Detection/Necks/FPN.cs
• src/ComputerVision/Detection/Necks/NeckBase.cs
• src/ComputerVision/Detection/Necks/PANet.cs
• src/Diffusion/Attention/MotionModule.cs
• src/Diffusion/Conditioning/CLIPTextConditioner.cs
• src/Diffusion/Control/IPAdapterModel.cs
• src/Diffusion/NoisePredictors/NoisePredictorBase.cs
• src/Diffusion/VAE/VAEModelBase.cs
• src/Interpretability/Explainers/InfluenceFunctionExplainer.cs
• src/LoRA/Adapters/AdaLoRAAdapter.cs

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/ComputerVision/Detection/Backbones/BackboneBase.cs (2)

289-304: 🧹 Nitpick | 🔵 Trivial

Consider making FeatureMap<T> immutable.

The public setters on Features, Stride, and Level allow callers to mutate feature maps after construction, which could lead to unintended side effects if references are shared across detection components (neck, head, etc.).

Optional: use init-only properties

 public class FeatureMap<T>
 {
-    public Tensor<T> Features { get; set; }
-    public int Stride { get; set; }
-    public int Level { get; set; }
+    public Tensor<T> Features { get; init; }
+    public int Stride { get; init; }
+    public int Level { get; init; }

     public FeatureMap(Tensor<T> features, int stride, int level)
     {
         Features = features;
         Stride = stride;
         Level = level;
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ComputerVision/Detection/Backbones/BackboneBase.cs` around lines 289 -
304, Make FeatureMap<T> immutable by removing public setters on Features,
Stride, and Level and exposing them as read-only properties set only by the
constructor (or switch to init-only properties if preferred). Update the
FeatureMap<T> constructor to assign the readonly properties and keep SpatialSize
and Channels computed from the Features property; ensure no other code mutates
Features, Stride, or Level after construction by adjusting callers that
currently set those properties to construct a new FeatureMap<T> instead.

---

279-281: ⚠️ Potential issue | 🔴 Critical

Blocking: DeepCopy uses MemberwiseClone — the same aliasing problem that required making CreateNewInstance abstract.

Lines 280-281 perform a shallow clone via MemberwiseClone(), which reuses _stem, stage lists, Conv2D/Dense/MultiHeadSelfAttention wrappers, and all nested tensors. Any mutation on the "copy" will also mutate the original model.

This is inconsistent with the CreateNewInstance fix (lines 196-205) where the remarks explicitly warn against MemberwiseClone for this exact reason. DeepCopy must perform a true deep copy or be made abstract to force concrete backbones to implement proper cloning.

Suggested fix: make DeepCopy abstract

-    public override IFullModel<T, Tensor<T>, Tensor<T>> DeepCopy()
-        => (BackboneBase<T>)MemberwiseClone();
+    /// <summary>
+    /// Creates a deep copy of this backbone with fully independent internal state.
+    /// Concrete backbones must construct a new instance and copy all nested tensors.
+    /// </summary>
+    public abstract override IFullModel<T, Tensor<T>, Tensor<T>> DeepCopy();

As per coding guidelines: "Production Readiness (CRITICAL - Flag as BLOCKING): Simplified implementations: Code that takes shortcuts... require immediate fix."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ComputerVision/Detection/Backbones/BackboneBase.cs` around lines 279 -
281, The DeepCopy override currently returns a shallow clone via MemberwiseClone
in DeepCopy(), causing shared references to _stem, stage lists and nested tensor
wrappers; either make DeepCopy abstract (like CreateNewInstance) to force
concrete backbones to implement correct cloning, or replace the MemberwiseClone
implementation with a true deep-copy: allocate a new BackboneBase<T> instance
(or use CreateNewInstance()), deep-copy _stem and each
stage/wrapper/Conv2D/Dense/MultiHeadSelfAttention and all tensors into new
instances, and ensure lists are newly created and populated so the copy and
original share no mutable objects; update DeepCopy() signature accordingly and
remove the MemberwiseClone usage.

♻️ Duplicate comments (1)

src/Diffusion/Conditioning/CLIPTextConditioner.cs (1)

105-117: ⚠️ Potential issue | 🔴 Critical

Blocking: the production path still uses a simplified transformer block.

This code still replaces CLIP self-attention and the MLP with single linear projections. Documenting the shortcut in XML comments does not make it production-ready for a core diffusion conditioner; this needs to route through the real attention/feed-forward stack instead.

As per coding guidelines: “Production Readiness (CRITICAL - Flag as BLOCKING) … Simplified implementations … require immediate fix.”

Also applies to: 335-370

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Diffusion/Conditioning/CLIPTextConditioner.cs` around lines 105 - 117,
The XML comment reveals the CLIPTextConditioner class uses a simplified
transformer block (linear "attention" and "MLP") which is not production-ready;
update CLIPTextConditioner to call the real attention and feed-forward
implementations instead of the single linear projections — locate the
transformer/block implementation used inside CLIPTextConditioner (the method(s)
responsible for per-block processing around the simplified block and the final
projection, and the code paths referenced again at the region corresponding to
lines ~335-370) and replace the linear projections with the project's canonical
multi-head self-attention and MLP modules (use the existing
Attention/MultiHeadAttention and FeedForward/MLP classes or factory methods),
preserve existing embedding, positional, layer-norm and residual wiring, and
ensure the attention mask is passed through to the real attention API.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/Diffusion/Conditioning/CLIPTextConditioner.cs`:
- Around line 99-102: The default Encode(Tensor<T> input) calls
EncodeText(input) without an attention mask which lets GetPooledEmbedding treat
padded tokens as real; update Encode and any other default/overload paths
(including usages around lines noted and Tokenize/TokenizeBatch) to produce or
propagate an attentionMask (ensure Tokenize/TokenizeBatch return masks for
padded rows) and call EncodeText with that mask (or create a mask inside Encode
when null by zeroing padded rows) so GetPooledEmbedding always receives a valid
attentionMask; refer to the methods Encode, EncodeText, GetPooledEmbedding,
Tokenize, TokenizeBatch and the attentionMask parameter when making the changes.

In `@src/Diffusion/NoisePredictors/NoisePredictorBase.cs`:
- Around line 79-95: The XML remarks for EnumerateLayers() are incorrect about
the default ownership: update the documentation in NoisePredictorBase (the
remarks around EnumerateLayers(), ReflectInstanceLayers and DisposeOnceGuard) to
state clearly that the default implementation returns
Enumerable.Empty<ILayer<T>>() so subclasses must opt in to owned-layer disposal
by overriding EnumerateLayers() (for example returning
ReflectInstanceLayers(this) or explicit yields) and that DisposeOnceGuard only
prevents double-dispose (it does not make an empty default safe for owned-layer
disposal); make the same correction in the corresponding duplicated remarks
(lines ~879-883) so the docs match the implementation.
- Around line 416-421: The default PredictNoiseWithEmbedding implementation
incorrectly tries to recover a timestep from a sinusoidal embedding
(timeEmbedding[0,0]) and therefore must not guess; change
PredictNoiseWithEmbedding to fail fast by throwing a clear exception (e.g.,
NotSupportedException or InvalidOperationException) that instructs implementers
to override PredictNoiseWithEmbedding when using GetTimestepEmbedding-based
embeddings, instead of calling PredictNoise with an invalid timestep; reference
PredictNoiseWithEmbedding, PredictNoise and GetTimestepEmbedding in the message
so inheritors know which method to implement/override.
- Around line 761-809: ComputeGradientsWithTape currently calls Forward(input)
which defaults to PredictNoise(input, 0), locking training to t=0; change
ComputeGradientsWithTape to accept a timestep (e.g., int timestep) and call
PredictNoise(input, timestep) (or overload ComputeGradientsWithTape with a
timestep parameter), and keep Forward as a convenience wrapper that delegates to
PredictNoise(input, 0) so existing subclasses aren't broken; update
callers/tests to pass the correct timestep when computing gradients so
timestep-conditional predictors receive the proper conditioning.

---

Outside diff comments:
In `@src/ComputerVision/Detection/Backbones/BackboneBase.cs`:
- Around line 289-304: Make FeatureMap<T> immutable by removing public setters
on Features, Stride, and Level and exposing them as read-only properties set
only by the constructor (or switch to init-only properties if preferred). Update
the FeatureMap<T> constructor to assign the readonly properties and keep
SpatialSize and Channels computed from the Features property; ensure no other
code mutates Features, Stride, or Level after construction by adjusting callers
that currently set those properties to construct a new FeatureMap<T> instead.
- Around line 279-281: The DeepCopy override currently returns a shallow clone
via MemberwiseClone in DeepCopy(), causing shared references to _stem, stage
lists and nested tensor wrappers; either make DeepCopy abstract (like
CreateNewInstance) to force concrete backbones to implement correct cloning, or
replace the MemberwiseClone implementation with a true deep-copy: allocate a new
BackboneBase<T> instance (or use CreateNewInstance()), deep-copy _stem and each
stage/wrapper/Conv2D/Dense/MultiHeadSelfAttention and all tensors into new
instances, and ensure lists are newly created and populated so the copy and
original share no mutable objects; update DeepCopy() signature accordingly and
remove the MemberwiseClone usage.

---

Duplicate comments:
In `@src/Diffusion/Conditioning/CLIPTextConditioner.cs`:
- Around line 105-117: The XML comment reveals the CLIPTextConditioner class
uses a simplified transformer block (linear "attention" and "MLP") which is not
production-ready; update CLIPTextConditioner to call the real attention and
feed-forward implementations instead of the single linear projections — locate
the transformer/block implementation used inside CLIPTextConditioner (the
method(s) responsible for per-block processing around the simplified block and
the final projection, and the code paths referenced again at the region
corresponding to lines ~335-370) and replace the linear projections with the
project's canonical multi-head self-attention and MLP modules (use the existing
Attention/MultiHeadAttention and FeedForward/MLP classes or factory methods),
preserve existing embedding, positional, layer-norm and residual wiring, and
ensure the attention mask is passed through to the real attention API.

🪄 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: 54e8e4f3-df6c-4500-befb-ba4aaa594a5d

📥 Commits

Reviewing files that changed from the base of the PR and between 06e3acf and cbb4237.

📒 Files selected for processing (26)

• src/ComputerVision/Detection/Backbones/BackboneBase.cs
• src/ComputerVision/Detection/Backbones/CSPDarknet.cs
• src/ComputerVision/Detection/Backbones/EfficientNet.cs
• src/ComputerVision/Detection/Backbones/ResNet.cs
• src/ComputerVision/Detection/Backbones/SwinTransformer.cs
• src/ComputerVision/Detection/Necks/BiFPN.cs
• src/ComputerVision/Detection/Necks/FPN.cs
• src/ComputerVision/Detection/Necks/NeckBase.cs
• src/ComputerVision/Detection/Necks/PANet.cs
• src/Diffusion/Attention/MotionModule.cs
• src/Diffusion/Conditioning/CLIPTextConditioner.cs
• src/Diffusion/NoisePredictors/NoisePredictorBase.cs
• src/Diffusion/VAE/AudioVAE.cs
• src/Diffusion/VAE/AutoencoderKL.cs
• src/Diffusion/VAE/Causal3DVAE.cs
• src/Diffusion/VAE/DeepCompressionVAE.cs
• src/Diffusion/VAE/EQVAEModel.cs
• src/Diffusion/VAE/ImprovedVideoVAE.cs
• src/Diffusion/VAE/LiteVAEModel.cs
• src/Diffusion/VAE/SDXLVAEModel.cs
• src/Diffusion/VAE/StandardVAE.cs
• src/Diffusion/VAE/TemporalInterpolationVAE.cs
• src/Diffusion/VAE/TemporalVAE.cs
• src/Diffusion/VAE/VAEModelBase.cs
• src/Interpretability/Explainers/InfluenceFunctionExplainer.cs
• src/LoRA/Adapters/AdaLoRAAdapter.cs