Skip to content

Fixed bugs in OptionalColumnTransform and added bool support #4815

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.

Sign up for GitHub

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
harishsk merged 2 commits into from
Feb 10, 2020
Merged

Fixed bugs in OptionalColumnTransform and added bool support #4815

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f69e618
- Cleaned up OnnxContext's initializer interface
harishsk Feb 8, 2020
81f62ba
Fixed doc issues pointed out by code review
harishsk Feb 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 86 additions & 15 deletions src/Microsoft.ML.Data/Model/Onnx/OnnxContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using Microsoft.ML.Data;

Expand Down Expand Up @@ -130,7 +131,16 @@ public OnnxNode CreateNode(string opType, string input, string output, string na
public abstract List<long> RetrieveShapeOrNull(string variableName);

/// <summary>
/// Call this function can declare a global float
/// Call this function to declare a global bool
/// </summary>
/// <param name="value">The boolean value which is going to be added</param>
/// <param name="name">A string used as a seed to generate this initializer's name in the ONNX graph.</param>
/// <param name="makeUniqueName">Whether a unique name should be picked for this initializer.</param>
/// <returns>The initializer's ONNX name</returns>
public abstract string AddInitializer(bool value, string name = null, bool makeUniqueName = true);

/// <summary>
/// Call this function to declare a global float
/// </summary>
/// <param name="value">The float number which is going to be added</param>
/// <param name="name">A string used as a seed to generate this initializer's name in the ONNX graph.</param>
Expand All @@ -139,16 +149,17 @@ public OnnxNode CreateNode(string opType, string input, string output, string na
public abstract string AddInitializer(float value, string name = null, bool makeUniqueName = true);

/// <summary>
/// Call this function can declare a global long
/// Call this function to declare a global float

@ganik ganik Feb 10, 2020 •
edited by harishsk
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

float [](start = 51, length = 5)

long? #Resolved

/// </summary>
/// <param name="value">The long number which is going to be added into the ONNX graph</param>
/// <param name="value">The float number which is going to be added</param>
/// <param name="type">The type of integer to be added, e.g. typeof(short). Use this for all integer types smaller than Int32</param>

@kere-nel kere-nel Feb 10, 2020 •
edited by harishsk
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the type inclusive of Int32s? #Resolved

/// <param name="name">A string used as a seed to generate this initializer's name in the ONNX graph.</param>
/// <param name="makeUniqueName">Whether a unique name should be picked for this initializer.</param>
/// <returns>The initializer's ONNX name</returns>
public abstract string AddInitializer(long value, string name = null, bool makeUniqueName = true);
public abstract string AddInitializer(int value, Type type, string name = null, bool makeUniqueName = true);

/// <summary>
/// Call this function can declare a global string
/// Call this function to declare a global string
/// </summary>
/// <param name="value">The string which is going to be added into the ONNX graph</param>
/// <param name="name">A string used as a seed to generate this initializer's name in the ONNX graph.</param>
Expand All @@ -157,43 +168,103 @@ public OnnxNode CreateNode(string opType, string input, string output, string na
public abstract string AddInitializer(string value, string name = null, bool makeUniqueName = true);

/// <summary>
/// Call this function can declare a global float tensor
/// Call this function to declare a global long
/// </summary>
/// <param name="value">The long number which is going to be added into the ONNX graph</param>
/// <param name="name">A string used as a seed to generate this initializer's name in the ONNX graph.</param>
/// <param name="makeUniqueName">Whether a unique name should be picked for this initializer.</param>
/// <returns>The initializer's ONNX name</returns>
public abstract string AddInitializer(long value, string name = null, bool makeUniqueName = true);

/// <summary>
/// Call this function to declare a global double
/// </summary>
/// <param name="value">The double number which is going to be added into the ONNX graph</param>
/// <param name="name">A string used as a seed to generate this initializer's name in the ONNX graph.</param>
/// <param name="makeUniqueName">Whether a unique name should be picked for this initializer.</param>
/// <returns>The initializer's ONNX name</returns>
public abstract string AddInitializer(double value, string name = null, bool makeUniqueName = true);

/// <summary>
/// Call this function to declare a global ulong or uint
/// </summary>
/// <param name="value">The long number which is going to be added into the ONNX graph</param>
/// <param name="isUint64">true if value contains a ulong value and false if it contains uint </param>
/// <param name="name">A string used as a seed to generate this initializer's name in the ONNX graph.</param>
/// <param name="makeUniqueName">Whether a unique name should be picked for this initializer.</param>
/// <returns>The initializer's ONNX name</returns>
public abstract string AddInitializer(ulong value, bool isUint64, string name = null, bool makeUniqueName = true);

/// <summary>
/// Call this function to declare a global bool tensor
/// </summary>
/// <param name="values">The boolean values which are going to be added into the ONNX graph</param>
/// <param name="dims">The shape of values</param>
/// <param name="name">A string used as a seed to generate this initializer's name in the ONNX graph.</param>
/// <param name="makeUniqueName">Whether a unique name should be picked for this initializer.</param>
/// <returns>The initializer's ONNX name</returns>
public abstract string AddInitializer(IEnumerable<bool> values, IEnumerable<long> dims, string name = null, bool makeUniqueName = true);

/// <summary>
/// Call this function to declare a global float tensor
/// </summary>
/// <param name="values">The floats which are going to be added into the ONNX graph</param>
/// <param name="dims">The shape that the floats</param>
/// <param name="dims">The shape of values</param>
/// <param name="name">A string used as a seed to generate this initializer's name in the ONNX graph.</param>
/// <param name="makeUniqueName">Whether a unique name should be picked for this initializer.</param>
/// <returns>The initializer's ONNX name</returns>
public abstract string AddInitializer(IEnumerable<float> values, IEnumerable<long> dims, string name = null, bool makeUniqueName = true);

/// <summary>
/// Call this function can declare a global long tensor
/// Call this function to declare a global long tensor

@kere-nel kere-nel Feb 10, 2020 •
edited by harishsk
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This conflicts with the type param description: "Use this for adding array initializers of integer types smaller than Int32" #Resolved

/// </summary>
/// <param name="values">The ints which are going to be added into the ONNX graph</param>
/// <param name="type">The type of ints which are going to be added into the ONNX graph, e.g. typeof(short). Use this for adding array initializers of integer types smaller than Int32</param>
/// <param name="dims">The shape of values</param>
/// <param name="name">A string used as a seed to generate this initializer's name in the ONNX graph.</param>
/// <param name="makeUniqueName">Whether a unique name should be picked for this initializer.</param>
/// <returns>The initializer's ONNX name</returns>
public abstract string AddInitializer(IEnumerable<int> values, Type type, IEnumerable<long> dims, string name = null, bool makeUniqueName = true);

/// <summary>
/// Call this function to declare a global string tensor
/// </summary>
/// <param name="values">The strings which are going to be added into the ONNX graph</param>
/// <param name="dims">The shape of values</param>
/// <param name="name">A string used as a seed to generate this initializer's name in the ONNX graph.</param>
/// <param name="makeUniqueName">Whether a unique name should be picked for this initializer.</param>
/// <returns>The initializer's ONNX name</returns>
public abstract string AddInitializer(IEnumerable<string> values, IEnumerable<long> dims, string name = null, bool makeUniqueName = true);

/// <summary>
/// Call this function to declare a global long tensor
/// </summary>
/// <param name="values">The longs which are going to be added into the ONNX graph</param>
/// <param name="dims">The shape that the floats</param>
/// <param name="dims">The shape of values</param>
/// <param name="name">A string used as a seed to generate this initializer's name in the ONNX graph.</param>
/// <param name="makeUniqueName">Whether a unique name should be picked for this initializer.</param>
/// <returns>The initializer's ONNX name</returns>
public abstract string AddInitializer(IEnumerable<long> values, IEnumerable<long> dims, string name = null, bool makeUniqueName = true);

/// <summary>
/// Call this function can declare a global double tensor
/// Call this function to declare a global double tensor
/// </summary>
/// <param name="values">The doubles which are going to be added into the ONNX graph</param>
/// <param name="dims">The shape that the doubles</param>
/// <param name="dims">The shape of values</param>
/// <param name="name">A string used as a seed to generate this initializer's name in the ONNX graph.</param>
/// <param name="makeUniqueName">Whether a unique name should be picked for this initializer.</param>
/// <returns>The initializer's ONNX name</returns>
public abstract string AddInitializer(IEnumerable<double> values, IEnumerable<long> dims, string name = null, bool makeUniqueName = true);

/// <summary>
/// Call this function can declare a global string tensor
/// Call this function to declare a global double tensor

@ganik ganik Feb 10, 2020 •
edited by harishsk
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

double [](start = 51, length = 6)

ulong? #Resolved

/// </summary>
/// <param name="values">The strings which are going to be added into the ONNX graph</param>
/// <param name="dims">The shape that the strings</param>
/// <param name="values">The unsigned integers which are going to be added into the ONNX graph</param>
/// <param name="isUint64">Set to true if values contain ulong values false if they contain uint values</param>
/// <param name="dims">The shape of values</param>
/// <param name="name">A string used as a seed to generate this initializer's name in the ONNX graph.</param>
/// <param name="makeUniqueName">Whether a unique name should be picked for this initializer.</param>
/// <returns>The initializer's ONNX name</returns>
public abstract string AddInitializer(IEnumerable<string> values, IEnumerable<long> dims, string name = null, bool makeUniqueName = true);
public abstract string AddInitializer(IEnumerable<ulong> values, bool isUint64, IEnumerable<long> dims, string name = null, bool makeUniqueName = true);
}
}
71 changes: 66 additions & 5 deletions src/Microsoft.ML.OnnxConverter/OnnxContextImpl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,13 +279,27 @@ public override List<long> RetrieveShapeOrNull(string variableName)
}

/// Adds constant tensor into the graph.
public override string AddInitializer(bool value, string name = null, bool makeUniqueName = true)
{
name = AddVariable(name ?? "bool", makeUniqueName);
_initializers.Add(OnnxUtils.MakeInt32(name, typeof(bool), value ? 1 : 0));
Comment thread
kere-nel marked this conversation as resolved.
return name;
}

public override string AddInitializer(float value, string name = null, bool makeUniqueName = true)
{
name = AddVariable(name ?? "float", makeUniqueName);
_initializers.Add(OnnxUtils.MakeFloat(name, value));
return name;
}

public override string AddInitializer(int value, Type type, string name = null, bool makeUniqueName = true)
{
name = AddVariable(name ?? "int32", makeUniqueName);
_initializers.Add(OnnxUtils.MakeInt32(name, type, value));
return name;
}

public override string AddInitializer(string value, string name = null, bool makeUniqueName = true)
{
name = AddVariable(name ?? "string", makeUniqueName);
Expand All @@ -300,6 +314,31 @@ public override string AddInitializer(long value, string name = null, bool makeU
return name;
}

public override string AddInitializer(double value, string name = null, bool makeUniqueName = true)
{
name = AddVariable(name ?? "double", makeUniqueName);
_initializers.Add(OnnxUtils.MakeDouble(name, value));
return name;
}

public override string AddInitializer(ulong value, bool isUint64, string name = null, bool makeUniqueName = true)
{
name = AddVariable(name ?? "uint64", makeUniqueName);
_initializers.Add(OnnxUtils.MakeUInt(name, isUint64, value));
return name;
}

public override string AddInitializer(IEnumerable<bool> values, IEnumerable<long> dims, string name = null, bool makeUniqueName = true)
{
_host.CheckValue(values, nameof(values));
if (dims != null)
_host.Check(dims.Aggregate((x, y) => x * y) == values.Count(), "Number of elements doesn't match tensor size");

name = AddVariable(name ?? "bools", makeUniqueName);
_initializers.Add(OnnxUtils.MakeInt32s(name, typeof(bool), values.Select(v => Convert.ToInt32(v)), dims));
return name;
}

public override string AddInitializer(IEnumerable<float> values, IEnumerable<long> dims, string name = null, bool makeUniqueName = true)
{
_host.CheckValue(values, nameof(values));
Expand All @@ -311,6 +350,28 @@ public override string AddInitializer(IEnumerable<float> values, IEnumerable<lon
return name;
}

public override string AddInitializer(IEnumerable<int> values, Type type, IEnumerable<long> dims, string name = null, bool makeUniqueName = true)
{
_host.CheckValue(values, nameof(values));
if (dims != null)
_host.Check(dims.Aggregate((x, y) => x * y) == values.Count(), "Number of elements doesn't match tensor size");

name = AddVariable(name ?? "int32s", makeUniqueName);
_initializers.Add(OnnxUtils.MakeInt32s(name, type, values, dims));
return name;
}

public override string AddInitializer(IEnumerable<string> values, IEnumerable<long> dims, string name = null, bool makeUniqueName = true)
{
_host.CheckValue(values, nameof(values));
if (dims != null)
_host.Check(dims.Aggregate((x, y) => x * y) == values.Count(), "Number of elements doesn't match tensor size");

name = AddVariable(name ?? "strings", makeUniqueName);
_initializers.Add(OnnxUtils.MakeStrings(name, values, dims));
return name;
}

public override string AddInitializer(IEnumerable<long> values, IEnumerable<long> dims, string name = null, bool makeUniqueName = true)
{
_host.CheckValue(values, nameof(values));
Expand All @@ -328,19 +389,19 @@ public override string AddInitializer(IEnumerable<double> values, IEnumerable<lo
if (dims != null)
_host.Check(dims.Aggregate((x, y) => x * y) == values.Count(), "Number of elements doesn't match tensor size");

name = AddVariable(name ?? "double", makeUniqueName);
_initializers.Add(OnnxUtils.MakeDouble(name, values, dims));
name = AddVariable(name ?? "doubles", makeUniqueName);
_initializers.Add(OnnxUtils.MakeDoubles(name, values, dims));
return name;
}

public override string AddInitializer(IEnumerable<string> values, IEnumerable<long> dims, string name = null, bool makeUniqueName = true)
public override string AddInitializer(IEnumerable<ulong> values, bool isUint64, IEnumerable<long> dims, string name = null, bool makeUniqueName = true)
{
_host.CheckValue(values, nameof(values));
if (dims != null)
_host.Check(dims.Aggregate((x, y) => x * y) == values.Count(), "Number of elements doesn't match tensor size");

name = AddVariable(name ?? "strings", makeUniqueName);
_initializers.Add(OnnxUtils.MakeStrings(name, values, dims));
name = AddVariable(name ?? "uints", makeUniqueName);
_initializers.Add(OnnxUtils.MakeUInts(name, isUint64, values, dims));
return name;
}

Expand Down
60 changes: 59 additions & 1 deletion src/Microsoft.ML.OnnxConverter/OnnxUtils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,8 +410,66 @@ public static TensorProto MakeInt64s(string name, IEnumerable<long> values, IEnu
return tensor;
}

// Make int32 and smaller integer types scalar in ONNX from native C# number
public static TensorProto MakeInt32(string name, Type type, int value)
{
var tensor = new TensorProto();
tensor.Name = name;
tensor.DataType = (int)ConvertToTensorProtoType(type);
tensor.Int32Data.Add(value);
return tensor;
}

// Make int32 and smaller integer types vector (i.e., 1-D tensor) with dims=null. Otherwise, dims is used as the shape of the produced tensor.
public static TensorProto MakeInt32s(string name, Type type, IEnumerable<int> values, IEnumerable<long> dims = null)
{
var tensor = new TensorProto();
tensor.Name = name;
tensor.DataType = (int)ConvertToTensorProtoType(type);
tensor.Int32Data.AddRange(values);
if (dims != null)
tensor.Dims.AddRange(dims);
else
tensor.Dims.Add(values.Count());
return tensor;
}

// Make ulong and uint integer types scalar in ONNX from native C# number
public static TensorProto MakeUInt(string name, bool isUint64, ulong value)
{
var tensor = new TensorProto();
tensor.Name = name;
tensor.DataType = (int)ConvertToTensorProtoType(isUint64 ? typeof(ulong) : typeof(uint));
tensor.Uint64Data.Add(value);
return tensor;
}

// Make ulong and uint integer vector (i.e., 1-D tensor) with dims=null. Otherwise, dims is used as the shape of the produced tensor.
public static TensorProto MakeUInts(string name, bool isUint64, IEnumerable<ulong> values, IEnumerable<long> dims = null)
{
var tensor = new TensorProto();
tensor.Name = name;
tensor.DataType = (int)ConvertToTensorProtoType(isUint64 ? typeof(ulong) : typeof(uint));
tensor.Uint64Data.AddRange(values);
if (dims != null)
tensor.Dims.AddRange(dims);
else
tensor.Dims.Add(values.Count());
return tensor;
}

// Make int32 and smaller integer types scalar in ONNX from native C# number
public static TensorProto MakeDouble(string name, double value)
{
var tensor = new TensorProto();
tensor.Name = name;
tensor.DataType = (int)TensorProto.Types.DataType.Double;
tensor.DoubleData.Add(value);
return tensor;
}

// Make double vector (i.e., 1-D tensor) with dims=null. Otherwise, dims is used as the shape of the produced tensor.
public static TensorProto MakeDouble(string name, IEnumerable<double> values, IEnumerable<long> dims = null)
public static TensorProto MakeDoubles(string name, IEnumerable<double> values, IEnumerable<long> dims = null)
{
var tensor = new TensorProto();
tensor.Name = name;
Expand Down
Loading