Skip to content

Replace whitelist terminology to allow list #5328

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/Microsoft.ML.AutoML/API/ExperimentBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,22 @@ public abstract class ExperimentBase<TMetrics, TExperimentSettings>

private readonly IChannel _logger;
private readonly TaskKind _task;
private readonly IEnumerable<TrainerName> _trainerWhitelist;
private readonly IEnumerable<TrainerName> _trainerAllowList;

internal ExperimentBase(MLContext context,
IMetricsAgent<TMetrics> metricsAgent,
OptimizingMetricInfo optimizingMetricInfo,
TExperimentSettings settings,
TaskKind task,
IEnumerable<TrainerName> trainerWhitelist)
IEnumerable<TrainerName> trainerAllowList)
{
Context = context;
MetricsAgent = metricsAgent;
OptimizingMetricInfo = optimizingMetricInfo;
Settings = settings;
_logger = ((IChannelProvider)context).Start("AutoML");
_task = task;
_trainerWhitelist = trainerWhitelist;
_trainerAllowList = trainerAllowList;
}

/// <summary>
Expand Down Expand Up @@ -312,7 +312,7 @@ private CrossValidationExperimentResult<TMetrics> ExecuteCrossVal(IDataView[] tr

// Execute experiment & get all pipelines run
var experiment = new Experiment<CrossValidationRunDetail<TMetrics>, TMetrics>(Context, _task, OptimizingMetricInfo, progressHandler,
Settings, MetricsAgent, _trainerWhitelist, columns, runner, _logger);
Settings, MetricsAgent, _trainerAllowList, columns, runner, _logger);
var runDetails = experiment.Execute();

var bestRun = GetBestCrossValRun(runDetails);
Expand Down Expand Up @@ -347,7 +347,7 @@ private ExperimentResult<TMetrics> Execute(ColumnInformation columnInfo,
{
// Execute experiment & get all pipelines run
var experiment = new Experiment<RunDetail<TMetrics>, TMetrics>(Context, _task, OptimizingMetricInfo, progressHandler,
Settings, MetricsAgent, _trainerWhitelist, columns, runner, _logger);
Settings, MetricsAgent, _trainerAllowList, columns, runner, _logger);
var runDetails = experiment.Execute();

var bestRun = GetBestRun(runDetails);
Expand Down
8 changes: 4 additions & 4 deletions src/Microsoft.ML.AutoML/Experiment/Experiment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ internal class Experiment<TRunDetail, TMetrics> where TRunDetail : RunDetail
private readonly IProgress<TRunDetail> _progressCallback;
private readonly ExperimentSettings _experimentSettings;
private readonly IMetricsAgent<TMetrics> _metricsAgent;
private readonly IEnumerable<TrainerName> _trainerWhitelist;
private readonly IEnumerable<TrainerName> _trainerAllowList;
private readonly DirectoryInfo _modelDirectory;
private readonly DatasetColumnInfo[] _datasetColumnInfo;
private readonly IRunner<TRunDetail> _runner;
Expand All @@ -32,7 +32,7 @@ public Experiment(MLContext context,
IProgress<TRunDetail> progressCallback,
ExperimentSettings experimentSettings,
IMetricsAgent<TMetrics> metricsAgent,
IEnumerable<TrainerName> trainerWhitelist,
IEnumerable<TrainerName> trainerAllowList,
DatasetColumnInfo[] datasetColumnInfo,
IRunner<TRunDetail> runner,
IChannel logger)
Expand All @@ -44,7 +44,7 @@ public Experiment(MLContext context,
_progressCallback = progressCallback;
_experimentSettings = experimentSettings;
_metricsAgent = metricsAgent;
_trainerWhitelist = trainerWhitelist;
_trainerAllowList = trainerAllowList;
_modelDirectory = GetModelDirectory(_experimentSettings.CacheDirectory);
_datasetColumnInfo = datasetColumnInfo;
_runner = runner;
Expand All @@ -63,7 +63,7 @@ public IList<TRunDetail> Execute()
// get next pipeline
var getPipelineStopwatch = Stopwatch.StartNew();
var pipeline = PipelineSuggester.GetNextInferredPipeline(_context, _history, _datasetColumnInfo, _task,
_optimizingMetricInfo.IsMaximizing, _experimentSettings.CacheBeforeTrainer, _trainerWhitelist);
_optimizingMetricInfo.IsMaximizing, _experimentSettings.CacheBeforeTrainer, _trainerAllowList);

var pipelineInferenceTimeInSeconds = getPipelineStopwatch.Elapsed.TotalSeconds;

Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.ML.AutoML/Experiment/RecipeInference.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ internal static class RecipeInference
/// </summary>
/// <returns>Array of viable learners.</returns>
public static IEnumerable<SuggestedTrainer> AllowedTrainers(MLContext mlContext, TaskKind task,
ColumnInformation columnInfo, IEnumerable<TrainerName> trainerWhitelist)
ColumnInformation columnInfo, IEnumerable<TrainerName> trainerAllowList)
Copy link
Contributor

@justinormont justinormont Jul 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@harishsk, @sharwell : This isn't a breaking change, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is a public API, then it could be a source-breaking change for someone who used explicitly named arguments. It wouldn't be a binary-breaking change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AutoML isn't part of ML.NET's stable API. So this should be okay I think. Is that correct @sharwell?


In reply to: 461252764 [](ancestors = 461252764)

Copy link
Contributor

@sharwell sharwell Jul 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't speak to what is or is not part of the current stable public API. If this is not part of the stable public API, then it seems like it would be an easy change to approve.

{
var trainerExtensions = TrainerExtensionCatalog.GetTrainers(task, trainerWhitelist, columnInfo);
var trainerExtensions = TrainerExtensionCatalog.GetTrainers(task, trainerAllowList, columnInfo);

var trainers = new List<SuggestedTrainer>();
foreach (var trainerExtension in trainerExtensions)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ public static SuggestedPipeline GetNextInferredPipeline(MLContext context,
TaskKind task,
bool isMaximizingMetric,
CacheBeforeTrainer cacheBeforeTrainer,
IEnumerable<TrainerName> trainerWhitelist = null)
IEnumerable<TrainerName> trainerAllowList = null)
{
var availableTrainers = RecipeInference.AllowedTrainers(context, task,
ColumnInformationUtil.BuildColumnInfo(columns), trainerWhitelist);
ColumnInformationUtil.BuildColumnInfo(columns), trainerAllowList);
var transforms = TransformInferenceApi.InferTransforms(context, task, columns).ToList();
var transformsPostTrainer = TransformInferenceApi.InferTransformsPostTrainer(context, task, columns).ToList();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public static ITrainerExtension GetTrainerExtension(TrainerName trainerName)
}

public static IEnumerable<ITrainerExtension> GetTrainers(TaskKind task,
IEnumerable<TrainerName> whitelist, ColumnInformation columnInfo)
IEnumerable<TrainerName> allowList, ColumnInformation columnInfo)
{
IEnumerable<ITrainerExtension> trainers;
if (task == TaskKind.BinaryClassification)
Expand Down Expand Up @@ -101,10 +101,10 @@ public static IEnumerable<ITrainerExtension> GetTrainers(TaskKind task,
throw new NotSupportedException($"unsupported machine learning task type {task}");
}

if (whitelist != null)
if (allowList != null)
{
whitelist = new HashSet<TrainerName>(whitelist);
trainers = trainers.Where(t => whitelist.Contains(GetTrainerName(t)));
allowList = new HashSet<TrainerName>(allowList);
trainers = trainers.Where(t => allowList.Contains(GetTrainerName(t)));
}

return trainers;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ public IStopWordsRemoverOptions StopWordsRemoverOptions
[Argument(ArgumentType.AtMostOnce, HelpText = "Column containing the transformed text tokens.", ShortName = "tokens,showtext,showTransformedText", SortOrder = 9)]
public string OutputTokensColumnName;

[Argument(ArgumentType.Multiple, HelpText = "A dictionary of whitelisted terms.", ShortName = "dict", NullName = "<None>", SortOrder = 10, Hide = true)]
[Argument(ArgumentType.Multiple, HelpText = "A dictionary of allowed terms.", ShortName = "dict", NullName = "<None>", SortOrder = 10, Hide = true)]
internal TermLoaderArguments Dictionary;

[TGUI(Label = "Word Gram Extractor")]
Expand Down
4 changes: 2 additions & 2 deletions test/BaselineOutput/Common/EntryPoints/core_manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -23843,7 +23843,7 @@
}
]
},
"Desc": "A dictionary of whitelisted terms.",
"Desc": "A dictionary of allowed terms.",
"Aliases": [
"dict"
],
Expand Down Expand Up @@ -30844,4 +30844,4 @@
]
}
]
}
}
10 changes: 5 additions & 5 deletions test/Microsoft.ML.AutoML.Tests/TrainerExtensionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -399,18 +399,18 @@ public void PublicToPrivateTrainerNamesNullTest()
}

[Fact]
public void AllowedTrainersWhitelistNullTest()
public void AllowedTrainersAllowListNullTest()
{
var trainers = RecipeInference.AllowedTrainers(new MLContext(1), TaskKind.BinaryClassification, new ColumnInformation(), null);
Assert.True(trainers.Any());
}

[Fact]
public void AllowedTrainersWhitelistTest()
public void AllowedTrainersAllowListTest()
{
var whitelist = new[] { TrainerName.AveragedPerceptronBinary, TrainerName.FastForestBinary };
var trainers = RecipeInference.AllowedTrainers(new MLContext(1), TaskKind.BinaryClassification, new ColumnInformation(), whitelist);
Assert.Equal(whitelist.Count(), trainers.Count());
var allowList = new[] { TrainerName.AveragedPerceptronBinary, TrainerName.FastForestBinary };
var trainers = RecipeInference.AllowedTrainers(new MLContext(1), TaskKind.BinaryClassification, new ColumnInformation(), allowList);
Assert.Equal(allowList.Count(), trainers.Count());
}
}
}