Skip to content

Do not pass validation set and metrics callback to Image Classification API. #4430

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.

Sign up for GitHub

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
codemzs merged 2 commits into from
Nov 1, 2019
Merged

Do not pass validation set and metrics callback to Image Classification API. #4430

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8c33614
Do not pass validation set to Image Classification API.
codemzs Nov 1, 2019
2c79621
remove metric callback since it is set to default in ImageClassificat…
codemzs Nov 1, 2019
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
3 changes: 0 additions & 3 deletions src/Microsoft.ML.AutoML/TrainerExtensions/MultiTrainerExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,9 +240,6 @@ public ITrainerEstimator CreateInstance(MLContext mlContext, IEnumerable<Sweepab
ColumnInformation columnInfo, IDataView validationSet)
{
var options = TrainerExtensionUtil.CreateOptions<Options>(null, columnInfo.LabelColumnName);
options.ValidationSet = validationSet;

@justinormont justinormont Nov 1, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are you also passing the validation dataset during CodeGen? If so, it should be removed there too.

@codemzs codemzs Nov 1, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, I'm not passing validations set at codegen.

@justinormont justinormont Nov 1, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please file a tracking issue for cleaning up the other code, and enabling early stopping.

We should discuss how to get early stopping alive. It will be useful.

Remaining issues:

  1. Wrong dataset split -- Current code is using test data during the training process
    To fix: must split off the training dataset instead
  2. Incorrect featurization -- Currently the code incorrectly creates a featurized dataset split -- the classes won't align in various cases giving an incorrect calculation of accuracy within DNNImageClassifier
    To fix: we don't know yet; bit hard to get right
  3. Code elegance -- Current code is punching a hole through ~20 functions to pass down the dataset split. I think there's a more elegant method
  4. CodeGen -- The CodeGen currently doesn't accept a dataset split for its EarlyStopping/etc; CodeGen should be able to recreate the same model

This current PR is a small work around for (1) and (2) by simply not passing the dataset. We should more fully address them in a follow-up.

@codemzs codemzs Nov 1, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

For 1 and 2, it is up to the caller to pass the right dataset, there is no issue in the ImageClassification API by itself. Right now Image Classification in AutoML is enabled with default parameters and sweeping is DISABLED, the reason being, even without sweeping it can give you good results, we have actually ran more than 50 benchmark to make this claim. I will be happy to discuss in person why I disagree with most of your comment but will open an issue for clean up.

@justinormont justinormont Nov 1, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

... there is no issue in the ImageClassification API by itself.

This is specifically addressing the DNNImageClassifier use in AutoML.

var logger = ((IChannelProvider)mlContext).Start(nameof(ImageClassificationExtension));
options.MetricsCallback = (ImageClassificationMetrics metric) => { logger.Trace(metric.ToString()); } ;

@justinormont justinormont Nov 1, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In the future, we should work out how to get status updates all the way to Model Builder and the CLI.

@codemzs codemzs Nov 1, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You are already getting the status updates. Image Classification API by default plumbs status updates to MLCONTEXT.LOG. The caller needs to read from there.,

return mlContext.MulticlassClassification.Trainers.ImageClassification(options);
}

Expand Down