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/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/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/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); + } + } +} 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() {