The bug
34 files under src/ construct their options object twice:
public DeepARModel(DeepAROptions<T>? options = null)
: base(options ?? new DeepAROptions<T>()) // line 93 — instance A
{
_options = options ?? new DeepAROptions<T>(); // line 95 — instance B
When the caller passes null, the ?? evaluates on both lines, so the base class and the derived class end up holding two different options objects. Whatever the base reads off its copy is not what _options holds, and a mutation through one is invisible to the other.
Why this is not theoretical
MatryoshkaEmbedding had the same shape and it was load-bearing. TransformerEmbeddingNetwork's constructor sets _embeddingDimension = _options.EmbeddingDimension, so with no options supplied the base built a TransformerEmbeddingOptions (768) while the derived class built a MatryoshkaEmbeddingOptions (1536). The base's copy is the one that sizes every layer and bounds EmbedResized, so a model documented and tested as 1536 wide was built 768 wide, and three MatryoshkaEmbeddingTests were failing on exactly that. Fixed in e578b030b9 on feature/options-surface-phase-7-audio; all 32 of those tests now pass.
Severity across the other 33 depends on whether the base reads anything off its copy, so many are currently latent — but they are one base-class change away from behaving like Matryoshka did, and none of them is intentional.
The symptom only appears on the options-null path. Any test that passes an explicit options object sees nothing wrong, which is why this survived.
The fix
Materialize once and hand the same instance to the base:
: base(options ??= new DeepAROptions<T>())
{
_options = options;
How to find them
for f in $(git grep -l -E ': base\(options \?\? new' -- 'src/**/*.cs'); do
grep -qE '_options = options \?\? new' "$f" && echo "$f"
done
The 34
src/Clustering/ dominates the list — AutoK/{GMeans,XMeans}, Density/{DBSCAN,HDBSCAN,MeanShift,OPTICS}, Hierarchical/{AgglomerativeClustering,BIRCH,BisectingKMeans}, Neural/SelfOrganizingMap, Partitioning/{AffinityPropagation,FuzzyCMeans,KMeans,KMedoids,MiniBatchKMeans}, Probabilistic/GaussianMixtureModel, SemiSupervised/{COPKMeans,SeededKMeans}, Spectral/SpectralClustering, Streaming/MiniBatchKMeans — plus src/TimeSeries/DeepARModel.cs and others. Run the command above for the current full list.
Found while adversarially reviewing #2130; all 34 are pre-existing on master and none was introduced by that PR.
The bug
34 files under
src/construct their options object twice:When the caller passes
null, the??evaluates on both lines, so the base class and the derived class end up holding two different options objects. Whatever the base reads off its copy is not what_optionsholds, and a mutation through one is invisible to the other.Why this is not theoretical
MatryoshkaEmbeddinghad the same shape and it was load-bearing.TransformerEmbeddingNetwork's constructor sets_embeddingDimension = _options.EmbeddingDimension, so with no options supplied the base built aTransformerEmbeddingOptions(768) while the derived class built aMatryoshkaEmbeddingOptions(1536). The base's copy is the one that sizes every layer and boundsEmbedResized, so a model documented and tested as 1536 wide was built 768 wide, and threeMatryoshkaEmbeddingTestswere failing on exactly that. Fixed ine578b030b9onfeature/options-surface-phase-7-audio; all 32 of those tests now pass.Severity across the other 33 depends on whether the base reads anything off its copy, so many are currently latent — but they are one base-class change away from behaving like Matryoshka did, and none of them is intentional.
The symptom only appears on the options-null path. Any test that passes an explicit options object sees nothing wrong, which is why this survived.
The fix
Materialize once and hand the same instance to the base:
How to find them
The 34
src/Clustering/dominates the list — AutoK/{GMeans,XMeans}, Density/{DBSCAN,HDBSCAN,MeanShift,OPTICS}, Hierarchical/{AgglomerativeClustering,BIRCH,BisectingKMeans}, Neural/SelfOrganizingMap, Partitioning/{AffinityPropagation,FuzzyCMeans,KMeans,KMedoids,MiniBatchKMeans}, Probabilistic/GaussianMixtureModel, SemiSupervised/{COPKMeans,SeededKMeans}, Spectral/SpectralClustering, Streaming/MiniBatchKMeans — plussrc/TimeSeries/DeepARModel.csand others. Run the command above for the current full list.Found while adversarially reviewing #2130; all 34 are pre-existing on
masterand none was introduced by that PR.