Skip to content

Restore OVA ability to preserve key names on predicted label #3101

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
merged 9 commits into from
Apr 1, 2019
Merged

Restore OVA ability to preserve key names on predicted label #3101

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
33a578e
Messy mess
Mar 26, 2019
a73b04e
Let's use training labels instead of slot names
Mar 27, 2019
fbbcfcf
use TrainingLabelValues in predict and metadata
Mar 27, 2019
d46ae57
Return slotnames and traininglabels if possible
Mar 28, 2019
2bb3ecc
Let's validate model
Mar 28, 2019
a61b914
also fix naive bayes for less than 4 features.
Mar 28, 2019
b68c4e5
use metakind
Mar 28, 2019
06cce1a
shuffle test around
Apr 1, 2019
10541d6
address comments
Apr 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
2 changes: 1 addition & 1 deletion src/Microsoft.ML.Data/Scorers/MulticlassClassificationScorer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ internal static bool CanWrap(ISchemaBoundMapper mapper, DataViewType labelNameTy
var scoreType = outSchema[scoreIdx].Type;

// Check that the type is vector, and is of compatible size with the score output.
return labelNameType is VectorDataViewType vectorType && vectorType.Size == scoreType.GetVectorSize() && vectorType.ItemType == TextDataViewType.Instance;
return labelNameType is VectorDataViewType vectorType && vectorType.Size == scoreType.GetVectorSize();
}

internal static ISchemaBoundMapper WrapCore<T>(IHostEnvironment env, ISchemaBoundMapper mapper, RoleMappedSchema trainSchema)
Expand Down
36 changes: 34 additions & 2 deletions test/Microsoft.ML.Tests/TrainerEstimators/MetalinearEstimators.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,36 @@ public void OVAUncalibrated()
Done();
}

/// <summary>
/// Test what OVA preserves key values for label.
/// </summary>
[Fact]
public void OvaKeyNames()
Copy link
Contributor

@TomFinley TomFinley Mar 27, 2019 •
edited by Ivanidzo4ka
Loading

Choose a reason for hiding this comment

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

OvaKeyNames [](start = 20, length = 11)

Just curious, I heard something fairly troubling from @eerhardt, could we test this sort of scenario still works for things other than OVA? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any tests which use TestEstimatorCore will compare metadata from expected and resulted output schema.
I've change AnnotationsForMulticlassScoreColumn to always have TrainingLabelValue, and slotNames if key was text type, so technically we check that everywhere where we use TestEstimatorCore

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

Copy link
Member

@eerhardt eerhardt Mar 27, 2019 •
edited by Ivanidzo4ka
Loading

Choose a reason for hiding this comment

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

Can we add this to FunctionalTests instead? Let's not keep adding tests that the product allows InternalsVisibleTo. That way we can test like a customer uses the product. #Resolved

{
var textLoaderOptions = new TextLoader.Options()
{
Columns = new[]
{ new TextLoader.Column("Label", DataKind.Single, 0),
new TextLoader.Column("Row", DataKind.Single, 1),
new TextLoader.Column("Column", DataKind.Single, 2),
},
HasHeader = true,
Separators = new[] { '\t' }
};
var textLoader = ML.Data.CreateTextLoader(textLoaderOptions);
var data = textLoader.Load(TestDatasets.trivialMatrixFactorization.trainFilename);

var ap = ML.BinaryClassification.Trainers.AveragedPerceptron();
var ova = ML.MulticlassClassification.Trainers.OneVersusAll(ap);

var pipeline = ML.Transforms.Conversion.MapValueToKey("Label")
.Append(ML.Transforms.Concatenate("Features", "Row", "Column"))
.Append(ova)
.Append(ML.Transforms.Conversion.MapKeyToValue("PredictedLabel"));

var model = pipeline.Fit(data);
Copy link
Member

@eerhardt eerhardt Mar 28, 2019 •
edited by Ivanidzo4ka
Loading

Choose a reason for hiding this comment

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

We should do something with the model to ensure it was created correctly. #Resolved

}

/// <summary>
/// Pairwise Coupling trainer
/// </summary>
Expand Down Expand Up @@ -83,12 +113,14 @@ public void MetacomponentsFeaturesRenamed()
var data = loader.Load(GetDataPath(TestDatasets.irisData.trainFilename));

var sdcaTrainer = ML.BinaryClassification.Trainers.SdcaNonCalibrated(
new SdcaNonCalibratedBinaryTrainer.Options {
new SdcaNonCalibratedBinaryTrainer.Options
{
LabelColumnName = "Label",
FeatureColumnName = "Vars",
MaximumNumberOfIterations = 100,
Shuffle = true,
NumberOfThreads = 1, });
NumberOfThreads = 1,
});

var pipeline = new ColumnConcatenatingEstimator(Env, "Vars", "SepalLength", "SepalWidth", "PetalLength", "PetalWidth")
.Append(new ValueToKeyMappingEstimator(Env, "Label"), TransformerScope.TrainTest)
Expand Down