Skip to content

Fix for MulticlassNaiveBayesTrainer export to Onnx #4928

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

Merged
merged 5 commits into from
Mar 11, 2020

Conversation

Lynx1820
Copy link
Contributor

  • Adding support for a batch input dimension
  • While ML.NET doesn't use this batch dimension, ORT does.

@Lynx1820 Lynx1820 requested a review from a team as a code owner March 10, 2020 19:48
Copy link
Contributor

@harishsk harishsk left a comment

Choose a reason for hiding this comment

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

🕐

var opType = "Greater";
ctx.CreateNode(opType, new[] { featureColumn, zero }, new[] { greaterOutput }, ctx.GetNodeName(opType), "");

opType = "Cast";
var isFeaturePresent = ctx.AddIntermediateVariable(null, "isFeaturePresent", true);
var node = ctx.CreateNode(opType, greaterOutput, isFeaturePresent, ctx.GetNodeName(opType), "");
var castOutput = ctx.AddIntermediateVariable(new VectorDataViewType(NumberDataViewType.Single, _featureHistogram[0].Length), "castOutput");
Copy link
Contributor

Choose a reason for hiding this comment

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

new VectorDataViewType(NumberDataViewType.Single, _featureHistogram[0].Length) [](start = 57, length = 78)

There seem to be about two or three shapes involved here. Can you please create the shapes in one place at the top and reuse the shapes?
It would make the code more readable and make it easier to follow along?

Copy link
Contributor

@harishsk harishsk left a comment

Choose a reason for hiding this comment

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

:shipit:

@Lynx1820 Lynx1820 merged commit ed481b6 into dotnet:master Mar 11, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Mar 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants