From 225c3f5b19b20e78fb00d28b9ed5dedc738caedf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 8 Sep 2026 09:19:11 +0000 Subject: [PATCH 1/4] Initial plan From 49fe5d63670252f8be87e3f6edbeec9dc0294bab Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 8 Sep 2026 11:51:06 +0000 Subject: [PATCH 2/4] Fix EstimatorChain null fallback and add regression coverage Co-authored-by: rosebyte <14963300+rosebyte@users.noreply.github.com> --- .../DataLoadSave/EstimatorChain.cs | 2 +- test/Microsoft.ML.Tests/CachingTests.cs | 28 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.ML.Data/DataLoadSave/EstimatorChain.cs b/src/Microsoft.ML.Data/DataLoadSave/EstimatorChain.cs index 05afb8d9eb..93ca453132 100644 --- a/src/Microsoft.ML.Data/DataLoadSave/EstimatorChain.cs +++ b/src/Microsoft.ML.Data/DataLoadSave/EstimatorChain.cs @@ -36,7 +36,7 @@ private EstimatorChain(IHostEnvironment env, IEstimator[] estimato _host = env?.Register(nameof(EstimatorChain)); _estimators = estimators ?? new IEstimator[0]; _scopes = scopes ?? new TransformerScope[0]; - LastEstimator = estimators.LastOrDefault() as IEstimator; + LastEstimator = _estimators.LastOrDefault() as IEstimator; _needCacheAfter = needCacheAfter ?? new bool[0]; Contracts.Assert((_host != null) == _needCacheAfter.Any(x => x)); diff --git a/test/Microsoft.ML.Tests/CachingTests.cs b/test/Microsoft.ML.Tests/CachingTests.cs index b54a02bb75..aa493c1355 100644 --- a/test/Microsoft.ML.Tests/CachingTests.cs +++ b/test/Microsoft.ML.Tests/CachingTests.cs @@ -4,9 +4,11 @@ using System; using System.Linq; +using System.Reflection; using System.Threading; using Microsoft.ML.Data; using Microsoft.ML.RunTests; +using Microsoft.ML.Runtime; using Xunit; using Xunit.Abstractions; @@ -69,6 +71,32 @@ public void CacheOnEmptyEstimatorChainTest() StringComparison.InvariantCultureIgnoreCase); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public void EstimatorChainAcceptsNullOrEmptyEstimators(bool useNull) + { + var constructor = typeof(EstimatorChain).GetConstructor( + BindingFlags.Instance | BindingFlags.NonPublic, null, + new[] { typeof(IHostEnvironment), typeof(IEstimator[]), typeof(TransformerScope[]), typeof(bool[]) }, null); + Assert.NotNull(constructor); + + var chain = (EstimatorChain)constructor.Invoke(new object[] + { + null, + useNull ? null : Array.Empty>(), + useNull ? null : Array.Empty(), + useNull ? null : Array.Empty() + }); + + Assert.Null(chain.LastEstimator); + var data = ML.Data.LoadFromEnumerable(new[] { new MyData() }); + Assert.Same(data, chain.Fit(data).Transform(data)); + + var estimator = ML.Transforms.CopyColumns("F1", "Features"); + Assert.Same(estimator, chain.Append(estimator).LastEstimator); + } + private void CacheOnEmptyEstimatorChain() { new EstimatorChain().AppendCacheCheckpoint(ML) From 1f8c021bb543bad1d72f0a0a47ec924dab8c4ecc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 8 Sep 2026 11:58:15 +0000 Subject: [PATCH 3/4] Fix TransformerChain null fallback and repeated enumeration Co-authored-by: rosebyte <14963300+rosebyte@users.noreply.github.com> --- .../DataLoadSave/TransformerChain.cs | 2 +- test/Microsoft.ML.Tests/CachingTests.cs | 28 ------ test/Microsoft.ML.Tests/ChainTests.cs | 94 +++++++++++++++++++ 3 files changed, 95 insertions(+), 29 deletions(-) create mode 100644 test/Microsoft.ML.Tests/ChainTests.cs diff --git a/src/Microsoft.ML.Data/DataLoadSave/TransformerChain.cs b/src/Microsoft.ML.Data/DataLoadSave/TransformerChain.cs index e11a894be0..75edee049b 100644 --- a/src/Microsoft.ML.Data/DataLoadSave/TransformerChain.cs +++ b/src/Microsoft.ML.Data/DataLoadSave/TransformerChain.cs @@ -88,7 +88,7 @@ public TransformerChain(IEnumerable transformers, IEnumerable 0) == (LastTransformer != null)); Contracts.Check(_transformers.Length == _scopes.Length); diff --git a/test/Microsoft.ML.Tests/CachingTests.cs b/test/Microsoft.ML.Tests/CachingTests.cs index aa493c1355..b54a02bb75 100644 --- a/test/Microsoft.ML.Tests/CachingTests.cs +++ b/test/Microsoft.ML.Tests/CachingTests.cs @@ -4,11 +4,9 @@ using System; using System.Linq; -using System.Reflection; using System.Threading; using Microsoft.ML.Data; using Microsoft.ML.RunTests; -using Microsoft.ML.Runtime; using Xunit; using Xunit.Abstractions; @@ -71,32 +69,6 @@ public void CacheOnEmptyEstimatorChainTest() StringComparison.InvariantCultureIgnoreCase); } - [Theory] - [InlineData(true)] - [InlineData(false)] - public void EstimatorChainAcceptsNullOrEmptyEstimators(bool useNull) - { - var constructor = typeof(EstimatorChain).GetConstructor( - BindingFlags.Instance | BindingFlags.NonPublic, null, - new[] { typeof(IHostEnvironment), typeof(IEstimator[]), typeof(TransformerScope[]), typeof(bool[]) }, null); - Assert.NotNull(constructor); - - var chain = (EstimatorChain)constructor.Invoke(new object[] - { - null, - useNull ? null : Array.Empty>(), - useNull ? null : Array.Empty(), - useNull ? null : Array.Empty() - }); - - Assert.Null(chain.LastEstimator); - var data = ML.Data.LoadFromEnumerable(new[] { new MyData() }); - Assert.Same(data, chain.Fit(data).Transform(data)); - - var estimator = ML.Transforms.CopyColumns("F1", "Features"); - Assert.Same(estimator, chain.Append(estimator).LastEstimator); - } - private void CacheOnEmptyEstimatorChain() { new EstimatorChain().AppendCacheCheckpoint(ML) diff --git a/test/Microsoft.ML.Tests/ChainTests.cs b/test/Microsoft.ML.Tests/ChainTests.cs new file mode 100644 index 0000000000..93391ca623 --- /dev/null +++ b/test/Microsoft.ML.Tests/ChainTests.cs @@ -0,0 +1,94 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Reflection; +using Microsoft.ML.Data; +using Microsoft.ML.RunTests; +using Microsoft.ML.Runtime; +using Microsoft.ML.Transforms; +using Xunit; +using Xunit.Abstractions; + +namespace Microsoft.ML.Tests +{ + public class ChainTests : TestDataPipeBase + { + public ChainTests(ITestOutputHelper helper) : base(helper) + { + } + + private class MyData + { + public float Feature { get; set; } + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void EstimatorChainAcceptsNullOrEmptyEstimators(bool useNull) + { + var constructor = typeof(EstimatorChain).GetConstructor( + BindingFlags.Instance | BindingFlags.NonPublic, null, + new[] { typeof(IHostEnvironment), typeof(IEstimator[]), typeof(TransformerScope[]), typeof(bool[]) }, null); + Assert.NotNull(constructor); + + var chain = (EstimatorChain)constructor.Invoke(new object[] + { + null, + useNull ? null : Array.Empty>(), + useNull ? null : Array.Empty(), + useNull ? null : Array.Empty() + }); + + Assert.Null(chain.LastEstimator); + var data = ML.Data.LoadFromEnumerable(new[] { new MyData() }); + Assert.Same(data, chain.Fit(data).Transform(data)); + + var estimator = ML.Transforms.CopyColumns("F1", "Feature"); + Assert.Same(estimator, chain.Append(estimator).LastEstimator); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void TransformerChainAcceptsNullOrEmptyTransformers(bool useNull) + { + var chain = new TransformerChain( + useNull ? null : Array.Empty(), + useNull ? null : Array.Empty()); + + Assert.Null(chain.LastTransformer); + Assert.Empty(chain); + var data = ML.Data.LoadFromEnumerable(new[] { new MyData() }); + Assert.Same(data, chain.Transform(data)); + Assert.Same(data.Schema, chain.GetOutputSchema(data.Schema)); + } + + [Fact] + public void TransformerChainEnumeratesTransformersOnce() + { + var data = ML.Data.LoadFromEnumerable(new[] { new MyData() }); + var first = ML.Transforms.CopyColumns("F1", "Feature").Fit(data); + var last = ML.Transforms.CopyColumns("F2", "F1").Fit(first.Transform(data)); + int enumerationCount = 0; + + IEnumerable GetTransformers() + { + enumerationCount++; + yield return first; + yield return last; + } + + var chain = new TransformerChain( + GetTransformers(), new[] { TransformerScope.Everything, TransformerScope.Everything }); + + Assert.Equal(1, enumerationCount); + Assert.Equal(new ITransformer[] { first, last }, chain); + Assert.Same(last, chain.LastTransformer); + Assert.Equal(typeof(float), chain.Transform(data).Schema["F2"].Type.RawType); + } + } +} From 12fa2bdb4e1c5bb4046753c698e8404f13885858 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 8 Sep 2026 12:15:48 +0000 Subject: [PATCH 4/4] Address recovered FastTree, LDA, and ensemble analyzer findings Co-authored-by: rosebyte <14963300+rosebyte@users.noreply.github.com> --- src/Microsoft.ML.Ensemble/PipelineEnsemble.cs | 2 +- src/Microsoft.ML.FastTree/BoostingFastTree.cs | 3 -- .../Text/LdaSingleBox.cs | 2 +- .../Text/LdaTransform.cs | 2 +- .../UnitTests/TestEntryPoints.cs | 40 +++++++++++++++++++ .../TrainerEstimators/TreeEstimators.cs | 30 ++++++++++++++ 6 files changed, 73 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.ML.Ensemble/PipelineEnsemble.cs b/src/Microsoft.ML.Ensemble/PipelineEnsemble.cs index 9ea6193762..b75f31acdd 100644 --- a/src/Microsoft.ML.Ensemble/PipelineEnsemble.cs +++ b/src/Microsoft.ML.Ensemble/PipelineEnsemble.cs @@ -667,7 +667,7 @@ private static int CheckKeyLabelColumnCore(IHostEnvironment env, PredictorMod throw env.Except("Label column of model {0} has different type than model 0", i); var mdType = labelCol.Annotations.Schema.GetColumnOrNull(AnnotationUtils.Kinds.KeyValues)?.Type; - if (!mdType.Equals(keyValuesType)) + if (!keyValuesType.Equals(mdType)) throw env.Except("Label column of model {0} has different key value type than model 0", i); labelCol.GetKeyValues(ref curLabelNames); if (!AreEqual(in labelNames, in curLabelNames)) diff --git a/src/Microsoft.ML.FastTree/BoostingFastTree.cs b/src/Microsoft.ML.FastTree/BoostingFastTree.cs index 16f6900fc3..0493f03551 100644 --- a/src/Microsoft.ML.FastTree/BoostingFastTree.cs +++ b/src/Microsoft.ML.FastTree/BoostingFastTree.cs @@ -41,9 +41,6 @@ private protected override void CheckOptions(IChannel ch) if (FastTreeTrainerOptions.CompressEnsemble && FastTreeTrainerOptions.WriteLastEnsemble) throw ch.Except("Ensemble compression cannot be done when forcing to write last ensemble (hl)"); - if (FastTreeTrainerOptions.NumberOfLeaves > 2 && FastTreeTrainerOptions.HistogramPoolSize > FastTreeTrainerOptions.NumberOfLeaves - 1) - throw ch.Except("Histogram pool size (ps) must be at least 2."); - if (FastTreeTrainerOptions.NumberOfLeaves > 2 && FastTreeTrainerOptions.HistogramPoolSize > FastTreeTrainerOptions.NumberOfLeaves - 1) throw ch.Except("Histogram pool size (ps) must be at most numLeaves - 1."); diff --git a/src/Microsoft.ML.Transforms/Text/LdaSingleBox.cs b/src/Microsoft.ML.Transforms/Text/LdaSingleBox.cs index 26a7d4105e..b02ee97aa0 100644 --- a/src/Microsoft.ML.Transforms/Text/LdaSingleBox.cs +++ b/src/Microsoft.ML.Transforms/Text/LdaSingleBox.cs @@ -139,7 +139,7 @@ public void AllocateModelMemory(int numTopic, int numVocab, long tableSize, long Contracts.Check(numVocab >= 0); Contracts.Check(tableSize >= 0); Contracts.Check(aliasTableSize >= 0); - LdaInterface.AllocateModelMemory(_engine, numVocab, numTopic, tableSize, aliasTableSize); + LdaInterface.AllocateModelMemory(_engine, numTopic, numVocab, tableSize, aliasTableSize); } public void AllocateDataMemory(int docNum, long corpusSize) diff --git a/src/Microsoft.ML.Transforms/Text/LdaTransform.cs b/src/Microsoft.ML.Transforms/Text/LdaTransform.cs index 6a0a152450..46113563ad 100644 --- a/src/Microsoft.ML.Transforms/Text/LdaTransform.cs +++ b/src/Microsoft.ML.Transforms/Text/LdaTransform.cs @@ -300,7 +300,7 @@ internal LdaState(IExceptionContext ectx, ModelLoadContext ctx) false, InfoEx.MaximumTokenCountPerDocument); - _ldaTrainer.AllocateModelMemory(_numVocab, InfoEx.NumberOfTopics, memBlockSize, aliasMemBlockSize); + _ldaTrainer.AllocateModelMemory(InfoEx.NumberOfTopics, _numVocab, memBlockSize, aliasMemBlockSize); for (int i = 0; i < _numVocab; i++) { diff --git a/test/Microsoft.ML.Core.Tests/UnitTests/TestEntryPoints.cs b/test/Microsoft.ML.Core.Tests/UnitTests/TestEntryPoints.cs index 6553f8d289..b2324c9ca7 100644 --- a/test/Microsoft.ML.Core.Tests/UnitTests/TestEntryPoints.cs +++ b/test/Microsoft.ML.Core.Tests/UnitTests/TestEntryPoints.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Reflection; using System.Text.RegularExpressions; using Microsoft.ML.Calibrators; using Microsoft.ML.Core.Tests.UnitTests; @@ -1537,6 +1538,45 @@ public void EntryPointCalibrate() Done(); } + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(2)] + public void PipelineEnsembleValidatesKeyValuesMetadata(int metadataSize) + { + PredictorModel CreateModel(int keyValuesCount) + { + var annotations = new DataViewSchema.Annotations.Builder(); + if (keyValuesCount > 0) + { + var values = new VBuffer(keyValuesCount, Enumerable.Range(0, keyValuesCount).ToArray()); + annotations.AddKeyValues(keyValuesCount, NumberDataViewType.Int32, + (ref VBuffer destination) => values.CopyTo(ref destination)); + } + + var schema = new DataViewSchema.Builder(); + schema.AddColumn("Label", new KeyDataViewType(typeof(uint), 2), annotations.ToAnnotations()); + var data = new EmptyDataView(Env, schema.ToSchema()); + return new PredictorModelImpl(Env, new RoleMappedData(data, label: "Label", feature: null), + data, new PriorModelParameters(Env, 0.5f)); + } + + var models = new[] { CreateModel(2), CreateModel(metadataSize) }; + if (metadataSize == 2) + { + Assert.NotNull(SchemaBindablePipelineEnsembleBase.Create(Env, models, new Average(Env), + AnnotationUtils.Const.ScoreColumnKind.BinaryClassification)); + } + else + { + var exception = Assert.Throws(() => + SchemaBindablePipelineEnsembleBase.Create(Env, models, new Average(Env), + AnnotationUtils.Const.ScoreColumnKind.BinaryClassification)); + var innerException = Assert.IsType(exception.InnerException); + Assert.Contains("Label column of model 1 has different key value type than model 0", innerException.Message); + } + } + [Fact] public void EntryPointPipelineEnsemble() { diff --git a/test/Microsoft.ML.Tests/TrainerEstimators/TreeEstimators.cs b/test/Microsoft.ML.Tests/TrainerEstimators/TreeEstimators.cs index b36dfa574a..c2d3630ea9 100644 --- a/test/Microsoft.ML.Tests/TrainerEstimators/TreeEstimators.cs +++ b/test/Microsoft.ML.Tests/TrainerEstimators/TreeEstimators.cs @@ -54,6 +54,36 @@ public void FastTreeBinaryEstimator() Done(); } + [Theory] + [InlineData(5, -1)] + [InlineData(5, 0)] + [InlineData(5, 1)] + [InlineData(5, 2)] + [InlineData(5, 4)] + [InlineData(5, 5)] + [InlineData(2, 1)] + public void FastTreeHistogramPoolSizeValidation(int numberOfLeaves, int histogramPoolSize) + { + var data = ML.Data.LoadFromEnumerable( + SamplesUtils.DatasetUtils.GenerateBinaryLabelFloatFeatureVectorFloatWeightSamples(100).ToList()); + var trainer = ML.BinaryClassification.Trainers.FastTree(new FastTreeBinaryTrainer.Options + { + NumberOfThreads = 1, + NumberOfTrees = 1, + NumberOfLeaves = numberOfLeaves, + HistogramPoolSize = histogramPoolSize, + MinimumExampleCountPerLeaf = 1, + }); + + if (numberOfLeaves > 2 && histogramPoolSize > numberOfLeaves - 1) + { + var exception = Assert.Throws(() => trainer.Fit(data)); + Assert.Contains("Histogram pool size (ps) must be at most numLeaves - 1.", exception.Message); + } + else + Assert.NotNull(trainer.Fit(data)); + } + [LightGBMFact] public void LightGBMBinaryEstimator() {