Use SweepablePipeline #6285

LittleLittleCloud · 2022-08-17T00:18:13Z

We are excited to review your PR.

So we can do the best job, please check:

There's a descriptive title that will make sense to other developers some time from now.
There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
You have included any necessary tests in the same PR.

Check out the spec for `SweepablePipeline` in #6218

SweepablePipeline is a combination of MultiModelPipeline and SweepableEstimatorPipeline, which supports a tree-like structure pipeline and support estimator-level search space using nested search space.

In another world, SweepablePipeline puts estimator candidates as part of its search space and makes it transparent to tuner. In this way, it decouples tuners from the detailed implementation of pipelines or trainers, and replacing them with Parameter and SearchSpace. The hyper-parameter optimization process, with the help of SweepablePipeline, can be simplified to the following 3 steps

ITuner sample parameter from search space
ITrialRunner train model and calculate score from parameter
ITuner update associated parameter with score.

Also, it provides a uniform way to create pipeline that includes multiple estimator candidates with search space.

And with this PR, the class that construct AutoML.Net Sweepable API is simplified to

ISweepable
- SweepableEstimator: Estimator with search space
- SweepablePipeline pipeline with search space

codecov · 2022-08-18T02:07:00Z

Codecov Report

Merging #6285 (4281116) into main (8589d25) will increase coverage by 0.05%.
The diff coverage is 99.65%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6285      +/-   ##
==========================================
+ Coverage   68.52%   68.58%   +0.05%     
==========================================
  Files        1170     1170              
  Lines      246931   247158     +227     
  Branches    25669    25675       +6     
==========================================
+ Hits       169220   169512     +292     
+ Misses      70961    70905      -56     
+ Partials     6750     6741       -9

Flag	Coverage Δ
Debug	`68.58% <99.65%> (+0.05%)`	⬆️
production	`63.01% <ø> (+0.01%)`	⬆️
test	`89.09% <99.65%> (+0.11%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
test/Microsoft.ML.AutoML.Tests/AutoFitTests.cs	`83.04% <98.64%> (+6.72%)`	⬆️
...t/Microsoft.ML.AutoML.Tests/AutoFeaturizerTests.cs	`92.45% <100.00%> (+0.96%)`	⬆️
...Microsoft.ML.AutoML.Tests/AutoMLExperimentTests.cs	`100.00% <100.00%> (ø)`
test/Microsoft.ML.AutoML.Tests/DatasetUtil.cs	`97.84% <100.00%> (+16.78%)`	⬆️
...icrosoft.ML.AutoML.Tests/SweepableExtensionTest.cs	`96.00% <100.00%> (+1.26%)`	⬆️
...ML.Transforms/Text/StopWordsRemovingTransformer.cs	`86.38% <0.00%> (-0.15%)`	⬇️
src/Microsoft.ML.Core/Data/ProgressReporter.cs	`77.94% <0.00%> (ø)`
src/Microsoft.ML.Data/Data/Conversion.cs	`79.98% <0.00%> (+0.09%)`	⬆️
src/Microsoft.ML.SearchSpace/SearchSpace.cs	`72.01% <0.00%> (+0.45%)`	⬆️
... and 6 more

…PI as extension

tarekgh · 2022-08-23T23:16:01Z

src/Microsoft.ML.AutoML/API/AutoMLExperimentExtension.cs

+
+        public static AutoMLExperiment SetBinaryClassificationMetric(this AutoMLExperiment experiment, BinaryClassificationMetric metric, string labelColumn = "label", string predictedColumn = "PredictedLabel")
+        {
+            var metricManager = new BinaryMetricManager(metric, predictedColumn, labelColumn);


predictedColumn, labelColumn

should we flip the order of these parameters to be consistent with the rest of APIs?

labelColumn should come before predictedColumn in order to be consistent with context.Binary.Evaluation api, I'll update BinaryMetricManager though

tarekgh · 2022-08-23T23:33:25Z

src/Microsoft.ML.AutoML/API/BinaryClassificationExperiment.cs


-            pipeline = pipeline.Append(Context.Auto().Featurizer(trainData, columnInformation, Features));
-            return pipeline.Append(Context.Auto().BinaryClassification(label, useSdca: useSdca, useFastTree: useFastTree, useLgbm: useLgbm, useLbfgs: uselbfgs, useFastForest: useFastForest, featureColumnName: Features));
+            throw new ArgumentException("IMetricManager must be BinaryMetricManager and IDatasetManager must be either TrainTestSplitDatasetManager or CrossValidationDatasetManager");


"IMetricManager must be BinaryMetricManager and IDatasetManager must be either TrainTestSplitDatasetManager or CrossValidationDatasetManager"

nit: I am seeing this message will not be clear if I see it thrown. Maybe you can modify it a little to tell something like,

$"The runner metric manager is of type {_metricManager.GetType()} which expected to be of type BinaryMetricManage"

tarekgh · 2022-08-23T23:35:20Z

src/Microsoft.ML.AutoML/API/MulticlassClassificationExperiment.cs

@@ -136,7 +141,8 @@ internal MulticlassClassificationExperiment(MLContext context, MulticlassExperim
        public override ExperimentResult<MulticlassClassificationMetrics> Execute(IDataView trainData, ColumnInformation columnInformation, IEstimator<ITransformer> preFeaturizer = null, IProgress<RunDetail<MulticlassClassificationMetrics>> progressHandler = null)
        {
            var label = columnInformation.LabelColumnName;
-            _experiment.SetEvaluateMetric(Settings.OptimizingMetric, label);
+            TrialResultMonitor<MulticlassClassificationMetrics> monitor = null;


TrialResultMonitor monitor = null;

nit: maybe better move this line down before _experiment.SetMonitor line?
This comment apply to similar places.

tarekgh · 2022-08-23T23:40:00Z

src/Microsoft.ML.AutoML/API/MulticlassClassificationExperiment.cs

+                }
+            }
+
+            throw new ArgumentException("IMetricManager must be MultiMetricManager and IDatasetManager must be either TrainTestSplitDatasetManager or CrossValidationDatasetManager");


"IMetricManager must be MultiMetricManager and IDatasetManager must be either TrainTestSplitDatasetManager or CrossValidationDatasetManager"

ditto.

tarekgh · 2022-08-23T23:44:20Z

        else

nit: you don't need the else here.

Refers to: src/Microsoft.ML.AutoML/AutoMLExperiment/AutoMLExperiment.cs:256 in 6de7519. [](commit_id = 6de7519, deletion_comment = False)

tarekgh · 2022-08-23T23:45:43Z

src/Microsoft.ML.AutoML/AutoMLExperiment/AutoMLExperiment.cs

                        throw;
                    }
+                    else


else

else not needed here.

That else will be hit when you get a training error but has a successful trial result(therefore _bestTrialResult is not null). In which case the current best result will be returned instead.

This is to avoid the case of losing all available trial results when encountering an unfatal error, like OOM or so. The more reliable way of doing that is, of course, detecting if exception from trial is fatal or not and continue training if the exception is not fatal. But in that case we need to cover all unfatal cases which is almost impossible and unnecessary. So as a step back, in order not to loss current training result, AutoMLExperiment simply 1) prints out exception and 2) return _currentBestTrial if there's any when encountering any exception. Only when there's no completed trial will AutoMLExperiment throws an exception.

The if block is throwing any way. so no need to have explicit else.

src/Microsoft.ML.AutoML/Tuner/EciCfoTuner.cs

tarekgh

I added minor comments. In general the change LGTM as you explained it to me.

LittleLittleCloud · 2022-08-25T04:02:47Z

/azp run

azure-pipelines · 2022-08-25T04:02:59Z

Azure Pipelines successfully started running 2 pipeline(s).

LittleLittleCloud added 3 commits August 16, 2022 13:34

replacing MultiModel Pipeline with SweepablePipeline

2722c86

remove pipeline in TrialSettings

b580667

clean up

192b266

ghost assigned LittleLittleCloud Aug 17, 2022

LittleLittleCloud added 9 commits August 17, 2022 11:33

implement regression && multiclass evaluation metric and add tests

cd92de5

remove tunarFactory

9ea3f43

rename EciCfoParameterProposer to EciCfoTuner

0c1d0d4

some clean up

bbe4858

Merge branch 'main' into u/xiaoyun/ISearchSpaceProposer

5892e9a

remove AutoMLExperimentSetting from TrialSetting

9b41c1a

disable lgbm trainer in test

6f4c95b

clean up

2eab5cb

renmame SetEvaluateMetricManager

928f98f

LittleLittleCloud changed the title ~~[wip] Use SweepablePipeline~~ Use SweepablePipeline Aug 18, 2022

LittleLittleCloud changed the title ~~Use SweepablePipeline~~ [wip] - Use SweepablePipeline Aug 18, 2022

LittleLittleCloud added 7 commits August 18, 2022 17:14

only keep back-to-bone function in AutoMLExperiment, and make other A…

c1bc1b9

…PI as extension

fix di

31362ff

clean up

4c65fa3

Merge branch 'main' into u/xiaoyun/ISearchSpaceProposer

2b17060

convert vector<bool> to vector<single?

c236830

disable log_base for lgbm

daa3db7

clean up

dec85d8

LittleLittleCloud changed the title ~~[wip] - Use SweepablePipeline~~ Use SweepablePipeline Aug 22, 2022

clean up

6de7519

LittleLittleCloud mentioned this pull request Aug 22, 2022

AutoML.Net improvements tracking down list #6145

Open

28 tasks

tarekgh self-requested a review August 22, 2022 20:00

tarekgh reviewed Aug 23, 2022

View reviewed changes

src/Microsoft.ML.AutoML/Tuner/EciCfoTuner.cs Outdated Show resolved Hide resolved

tarekgh approved these changes Aug 23, 2022

View reviewed changes

LittleLittleCloud and others added 7 commits August 24, 2022 11:36

fix comment and add comment in AutoMLExtension

ce58257

change namespace for AutoMLExtension

df379c5

fix build error

6be0a48

fix tests

c4b51f6

fix test

6c201a8

fix test

6834273

Update job-template.yml

4281116

LittleLittleCloud merged commit 9652e59 into dotnet:main Aug 25, 2022

ghost locked as resolved and limited conversation to collaborators Sep 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use SweepablePipeline #6285

Use SweepablePipeline #6285

LittleLittleCloud commented Aug 17, 2022 •

edited

Loading

codecov bot commented Aug 18, 2022 •

edited

Loading

tarekgh Aug 23, 2022

LittleLittleCloud Aug 24, 2022

LittleLittleCloud Aug 24, 2022

tarekgh Aug 23, 2022

LittleLittleCloud Aug 24, 2022

tarekgh Aug 23, 2022 •

edited

Loading

LittleLittleCloud Aug 24, 2022

tarekgh Aug 23, 2022

LittleLittleCloud Aug 24, 2022

tarekgh commented Aug 23, 2022

tarekgh Aug 23, 2022

LittleLittleCloud Aug 24, 2022

tarekgh Aug 24, 2022

LittleLittleCloud Aug 24, 2022

tarekgh left a comment

LittleLittleCloud commented Aug 25, 2022

azure-pipelines bot commented Aug 25, 2022

Use SweepablePipeline #6285

Use SweepablePipeline #6285

Conversation

LittleLittleCloud commented Aug 17, 2022 • edited Loading

Check out the spec for SweepablePipeline in #6218

codecov bot commented Aug 18, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Resolved

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Resolved

tarekgh Aug 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Resolved

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Resolved

tarekgh commented Aug 23, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarekgh left a comment

Choose a reason for hiding this comment

LittleLittleCloud commented Aug 25, 2022

azure-pipelines bot commented Aug 25, 2022

LittleLittleCloud commented Aug 17, 2022 •

edited

Loading

Check out the spec for `SweepablePipeline` in #6218

codecov bot commented Aug 18, 2022 •

edited

Loading

tarekgh Aug 23, 2022 •

edited

Loading