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

Fixes OneHotEncoding Issue #4974

Merged
merged 8 commits into from Mar 26, 2020
Merged

Fixes OneHotEncoding Issue #4974

merged 8 commits into from Mar 26, 2020

Conversation

@Lynx1820
Copy link
Member

Lynx1820 commented Mar 25, 2020

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
@Lynx1820 Lynx1820 requested a review from dotnet/mlnet-core as a code owner Mar 25, 2020
@Lynx1820 Lynx1820 changed the title Onnx sdt Fixes OneHotEncoding Issue Mar 25, 2020
}
CheckEquality(subDir, onnxTextName);

This comment has been minimized.

Copy link
@Lynx1820

Lynx1820 Mar 25, 2020

Author Member

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

@@ -1770,7 +1766,6 @@ private void CompareResults(string leftColumnName, string rightColumnName, IData
var rightColumn = right.Schema[rightColumnName];
var leftType = leftColumn.Type.GetItemType();
var rightType = rightColumn.Type.GetItemType();
Assert.Equal(leftType, rightType);

This comment has been minimized.

Copy link
@Lynx1820

Lynx1820 Mar 25, 2020

Author Member

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

@Lynx1820 Lynx1820 requested review from harishsk and ganik Mar 26, 2020
@ganik
ganik approved these changes Mar 26, 2020
Copy link
Member

ganik left a comment

:shipit:

Copy link
Member

harishsk left a comment

:shipit:

@ganik ganik merged commit 88ed4d9 into dotnet:master Mar 26, 2020
17 checks passed
17 checks passed
MachineLearning-CI Build #20200325.16 succeeded
Details
MachineLearning-CI (Centos_x64_NetCoreApp31 Debug_Build) Centos_x64_NetCoreApp31 Debug_Build succeeded
Details
MachineLearning-CI (Centos_x64_NetCoreApp31 Release_Build) Centos_x64_NetCoreApp31 Release_Build succeeded
Details
MachineLearning-CI (MacOS_x64_NetCoreApp21 Debug_Build) MacOS_x64_NetCoreApp21 Debug_Build succeeded
Details
MachineLearning-CI (MacOS_x64_NetCoreApp21 Release_Build) MacOS_x64_NetCoreApp21 Release_Build succeeded
Details
MachineLearning-CI (Ubuntu_x64_NetCoreApp21 Debug_Build) Ubuntu_x64_NetCoreApp21 Debug_Build succeeded
Details
MachineLearning-CI (Ubuntu_x64_NetCoreApp21 Release_Build) Ubuntu_x64_NetCoreApp21 Release_Build succeeded
Details
MachineLearning-CI (Windows_x64_NetCoreApp21 Debug_Build) Windows_x64_NetCoreApp21 Debug_Build succeeded
Details
MachineLearning-CI (Windows_x64_NetCoreApp21 Release_Build) Windows_x64_NetCoreApp21 Release_Build succeeded
Details
MachineLearning-CI (Windows_x64_NetCoreApp31 Debug_Build) Windows_x64_NetCoreApp31 Debug_Build succeeded
Details
MachineLearning-CI (Windows_x64_NetCoreApp31 Release_Build) Windows_x64_NetCoreApp31 Release_Build succeeded
Details
MachineLearning-CI (Windows_x64_NetFx461 Debug_Build) Windows_x64_NetFx461 Debug_Build succeeded
Details
MachineLearning-CI (Windows_x64_NetFx461 Release_Build) Windows_x64_NetFx461 Release_Build succeeded
Details
MachineLearning-CI (Windows_x86_NetCoreApp21 Debug_Build) Windows_x86_NetCoreApp21 Debug_Build succeeded
Details
MachineLearning-CI (Windows_x86_NetCoreApp21 Release_Build) Windows_x86_NetCoreApp21 Release_Build succeeded
Details
WIP Ready for review
Details
license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.