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

Adds PriorTrainer Onnx conversion #4515

Open
wants to merge 1 commit into
base: master
from

Conversation

@Lynx1820
Copy link
Member

Lynx1820 commented Dec 3, 2019

  1. Adding Onnx support for PriorTrainer
    To follow the onnx construction pattern of other binary classifiers, this trainer uses the probability to predict labels, instead of score
@Lynx1820 Lynx1820 requested a review from dotnet/mlnet-core as a code owner Dec 3, 2019
@Lynx1820 Lynx1820 requested review from harishsk, KsenijaS and ganik Dec 3, 2019
Contracts.Assert(Utils.Size(outputNames) == 3); // Predicted Label, Score and Probablity.

// Prior doesn't have a feature column and uses the training label column to determine predicted labels
if (mapper.ToString().EndsWith("PriorModelParameters")) {

This comment has been minimized.

Copy link
@harishsk

harishsk Dec 3, 2019

Member

I know I had asked you to explore the option of checking the mapper type here but I didn't mean check it as a string. I meant check the type directly (using typeof()). But now I don't think that is good idea either because we might introduce a bad dependency.
I think the better approach is to explicitly handle two cases (if the feature column is present and otherwise).

// Prior doesn't have a feature column and uses the training label column to determine predicted labels
if (mapper.ToString().EndsWith("PriorModelParameters")) {
var labelColumnName = schema.Schema[0].Name;
return mapper.SaveAsOnnx(ctx, outputNames, ctx.GetVariableName(labelColumnName));

This comment has been minimized.

Copy link
@harishsk

harishsk Dec 3, 2019

Member

Can you use the same pattern as schema.Feature.HasValue? (schema.Label.HasValue)?

This comment has been minimized.

Copy link
@Lynx1820

Lynx1820 Dec 3, 2019

Author Member

I can check whether the name is null, but the schema itself doesn't have the label name initialized. BinaryPredictionTransformer only populates the schema's feature column.

Copy link
Member

harishsk left a comment

🕐

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