Do not pass validation set and metrics callback to Image Classification API. - #4430
Conversation
| ColumnInformation columnInfo, IDataView validationSet) | ||
| { | ||
| var options = TrainerExtensionUtil.CreateOptions<Options>(null, columnInfo.LabelColumnName); | ||
| options.ValidationSet = validationSet; |
There was a problem hiding this comment.
Are you also passing the validation dataset during CodeGen? If so, it should be removed there too.
There was a problem hiding this comment.
No, I'm not passing validations set at codegen.
codemzs
changed the title
| ColumnInformation columnInfo, IDataView validationSet) | ||
| { | ||
| var options = TrainerExtensionUtil.CreateOptions<Options>(null, columnInfo.LabelColumnName); | ||
| options.ValidationSet = validationSet; |
There was a problem hiding this comment.
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:
- Wrong dataset split -- Current code is using test data during the training process
To fix: must split off the training dataset instead - 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 - 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
- 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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
... there is no issue in the ImageClassification API by itself.
This is specifically addressing the DNNImageClassifier use in AutoML.
| var options = TrainerExtensionUtil.CreateOptions<Options>(null, columnInfo.LabelColumnName); | ||
| options.ValidationSet = validationSet; | ||
| var logger = ((IChannelProvider)mlContext).Start(nameof(ImageClassificationExtension)); | ||
| options.MetricsCallback = (ImageClassificationMetrics metric) => { logger.Trace(metric.ToString()); } ; |
There was a problem hiding this comment.
In the future, we should work out how to get status updates all the way to Model Builder and the CLI.
There was a problem hiding this comment.
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.,
There was a problem hiding this comment.
LGTM; if you haven't please add a tracking issue for the items listed in #4430 (comment)
…on API. (dotnet#4430) * Do not pass validation set to Image Classification API. * remove metric callback since it is set to default in ImageClassification Trainer.
…on API. (dotnet#4430) * Do not pass validation set to Image Classification API. * remove metric callback since it is set to default in ImageClassification Trainer. resolve conflict clean up code
ghost
locked as resolved and limited conversation to collaborators
2 participants
No description provided.