Skip to content

Conversation

@kere-nel
Copy link
Contributor

OneHotEncoding always increases the dimension by one, which is causing issues with Nimbus.
See: microsoft/NimbusML#472

  • Added shape info to make models more easy to read
  • Replaced ReduceSum with Squeeze (makes more sense imo)
  • Isolated KeyToVector test to verify several outputKind modes - Skipping OneHotEncodingEstimator.OutputKind.Binary for now, since this mode is not working and it'll require implementation of KeyToBinaryVector

@kere-nel kere-nel requested a review from a team as a code owner March 25, 2020 23:38
@kere-nel kere-nel changed the title Onnx sdt Fixes OneHotEncoding Issue Mar 25, 2020
CompareSelectedColumns<bool>("PredictedLabel", "PredictedLabel", transformedData, onnxResult);
CompareResults("F2", "F2", transformedData, onnxResult);
}
CheckEquality(subDir, onnxTextName);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing the pipeline to isolate transformer. OneHotEncoding is already used in a pipeline within RemoveVariablesInPipelineTest.

var rightColumn = right.Schema[rightColumnName];
var leftType = leftColumn.Type.GetItemType();
var rightType = rightColumn.Type.GetItemType();
Assert.Equal(leftType, rightType);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Review: Types don't necessarily have to be equal. For example, an ML.NET type can be Key{uint32} and uint32 in onnx.

@kere-nel kere-nel requested review from ganik and harishsk March 26, 2020 00:03
Copy link
Member

@ganik ganik left a comment

Choose a reason for hiding this comment

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

:shipit:

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:

@ganik ganik merged commit 88ed4d9 into dotnet:master Mar 26, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Mar 18, 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.

3 participants