Skip to content
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

Allow only binary classifiers in OVA #2949

Merged
merged 1 commit into from Mar 14, 2019

Conversation

Projects
None yet
3 participants
@ganik
Copy link
Member

ganik commented Mar 13, 2019

fixes #2920

Gani Nazirov
@@ -628,7 +628,7 @@ private static ICalibratorTrainer GetCalibratorTrainerOrThrow(IExceptionContext
/// <param name="useProbabilities">Use probabilities (vs. raw outputs) to identify top-score category.</param>
/// <typeparam name="TModel">The type of the model. This type parameter will usually be inferred automatically from <paramref name="binaryEstimator"/>.</typeparam>
public static OneVersusAllTrainer OneVersusAll<TModel>(this MulticlassClassificationCatalog.MulticlassClassificationTrainers catalog,
ITrainerEstimator<ISingleFeaturePredictionTransformer<TModel>, TModel> binaryEstimator,
ITrainerEstimator<BinaryPredictionTransformer<TModel>, TModel> binaryEstimator,

This comment has been minimized.

@rogancarr

rogancarr Mar 14, 2019

Contributor

BinaryPredictionTransformer [](start = 30, length = 27)

Might not be so easy because any given BinaryPredictionTransformer might not be an ISingleFeaturePredictionTransformer; from what I understand, OVA is currently limited to learners with single feature vectors (e.g. not FieldAwareFactorizationMachines). #Resolved

This comment has been minimized.

@ganik

ganik Mar 14, 2019

Author Member

Here is the definition for BinaryPredictionTransformer:

/// <summary>
/// Base class for the <see cref="ISingleFeaturePredictionTransformer{TModel}"/> working on binary classification tasks.
/// </summary>
/// <typeparam name="TModel">An implementation of the <see cref="IPredictorProducing{TResult}"/></typeparam>
public sealed class BinaryPredictionTransformer<TModel> : SingleFeaturePredictionTransformerBase<TModel>

so it looks like its a base transformer class for all ISingleFeaturePredictionTransformer and thats exactly what we need.


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

@TomFinley
Copy link
Contributor

TomFinley left a comment

Thanks @ganik! This should help prevent people from feeding in the "wrong" types of models I hope.

@TomFinley TomFinley merged commit 6a4df7c into dotnet:master Mar 14, 2019

2 of 3 checks passed

MachineLearning-CodeCoverage #20190313.46 failed
Details
MachineLearning-CI #20190313.46 succeeded
Details
license/cla All CLA requirements met.
@rogancarr

This comment has been minimized.

Copy link
Contributor

rogancarr commented Mar 14, 2019

Broken build ! I'm fixing.

@rogancarr rogancarr referenced this pull request Mar 14, 2019

Merged

Updating OVA tests #2956

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.