Skip to content

Add support for combining hashes in vector columns to HashingTransformer #4828

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
yaeldekel merged 13 commits into from
May 18, 2020
Merged

Add support for combining hashes in vector columns to HashingTransformer #4828

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
03fbcf0
Add "Combine" option.
yaeldMS Feb 10, 2020
3d332ce
Add tests.
yaeldMS Feb 12, 2020
ea4a0b8
Add tests, and change data splitting APIs to use the new functionalit…
yaeldMS Feb 17, 2020
08bb1e5
Address code review comment, change missing value handling and add a …
yaeldMS Feb 25, 2020
d48a3e8
Fix hashing tests.
yaeldMS Feb 26, 2020
7868a52
Add test for backwards compatibility of model format.
yaeldMS Feb 28, 2020
f9c13aa
Fix build breaks and tests after rebase.
yaeldMS May 6, 2020
d315999
Combine V2 and V1 implementations of IHashers into one.
yaeldMS May 6, 2020
ef735d9
Update SaveAsOnnx.
yaeldMS May 6, 2020
d2c2d6a
fix baseline
yaeldMS May 11, 2020
d05dcf0
More unit test fixes
yaeldMS May 12, 2020
c5a140f
Code review comments.
yaeldMS May 14, 2020
e6bb9d8
Add back the back-compat test.
yaeldMS May 14, 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
8 changes: 4 additions & 4 deletions src/Microsoft.ML.Core/Utilities/Hashing.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ public static uint HashSequence(uint[] sequence, int min, int lim)

/// <summary>
/// Combines the given hash value with a uint value, using the murmur hash 3 algorithm.
/// Make certain to also use <see cref="MixHash"/> on the final hashed value, if you
/// depend upon having distinct bits.
/// Make certain to also use <see cref="MixHash(uint)"/> or <see cref="MixHash(uint, int)"/> on the final hashed value,
/// if you depend upon having distinct bits.
/// </summary>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static uint MurmurRound(uint hash, uint chunk)
Expand Down Expand Up @@ -257,7 +257,7 @@ public static uint MurmurHashV2(uint hash, ReadOnlySpan<char> span, bool toUpper
}

// Final mixing ritual for the hash.
hash = MixHashV2(hash, len);
hash = MixHash(hash, len);

return hash;
}
Expand Down Expand Up @@ -373,7 +373,7 @@ public static uint MixHash(uint hash)
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static uint MixHashV2(uint hash, int len)
public static uint MixHash(uint hash, int len)
{
hash ^= (uint)len;
hash ^= hash >> 16;
Expand Down
13 changes: 8 additions & 5 deletions src/Microsoft.ML.Data/Commands/CrossValidationCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,11 +328,14 @@ private string GetSplitColumn(IChannel ch, IDataView input, ref IDataView output
{
ch.Info("Hashing the stratification column");
var origStratCol = stratificationColumn;
int tmp;
int inc = 0;
while (input.Schema.TryGetColumnIndex(stratificationColumn, out tmp))
stratificationColumn = string.Format("{0}_{1:000}", origStratCol, ++inc);
output = new HashingEstimator(Host, origStratCol, stratificationColumn, 30).Fit(input).Transform(input);
stratificationColumn = input.Schema.GetTempColumnName("strat");

// HashingEstimator currently handles all primitive types except for DateTime, DateTimeOffset and TimeSpan.
var itemType = type.GetItemType();
if (itemType is DateTimeDataViewType || itemType is DateTimeOffsetDataViewType || itemType is TimeSpanDataViewType)
input = new TypeConvertingTransformer(Host, origStratCol, DataKind.Int64, origStratCol).Transform(input);

output = new HashingEstimator(Host, stratificationColumn, origStratCol, 30).Fit(input).Transform(input);
}

@harishsk harishsk Mar 3, 2020

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.

I realize the previous code too used 30 bits. But the default seems to be 31 bits. Any reason why we have 30 here? If there is a specific reason for 30 bits, can you please add a note to that in the code?

@yaeldekel yaeldekel May 14, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I am actually not sure why 30 was chosen. If you think it's important I can try changing it back to 31.

In reply to: 386729669 [](ancestors = 386729669)

@harishsk harishsk May 14, 2020

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.

I have no reason to say it should change to 31 either :-). But since it is is used in a couple of places, can you please make it a named constant?

In reply to: 425102125 [](ancestors = 425102125,386729669)

}

Expand Down
21 changes: 10 additions & 11 deletions src/Microsoft.ML.Data/DataLoadSave/DataOperationsCatalog.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,19 +511,18 @@ internal static void EnsureGroupPreservationColumn(IHostEnvironment env, ref IDa
var type = data.Schema[stratCol].Type;
if (!RangeFilter.IsValidRangeFilterColumnType(env, type))
{
// Hash the samplingKeyColumn.
// REVIEW: this could currently crash, since Hash only accepts a limited set
// of column types. It used to be HashJoin, but we should probably extend Hash
// instead of having two hash transformations.
var origStratCol = samplingKeyColumn;
samplingKeyColumn = data.Schema.GetTempColumnName(samplingKeyColumn);
HashingEstimator.ColumnOptionsInternal columnOptions;
if (seed.HasValue)
columnOptions = new HashingEstimator.ColumnOptionsInternal(samplingKeyColumn, origStratCol, 30, (uint)seed.Value);
else if (((ISeededEnvironment)env).Seed.HasValue)
columnOptions = new HashingEstimator.ColumnOptionsInternal(samplingKeyColumn, origStratCol, 30, (uint)((ISeededEnvironment)env).Seed.Value);
else
columnOptions = new HashingEstimator.ColumnOptionsInternal(samplingKeyColumn, origStratCol, 30);
// HashingEstimator currently handles all primitive types except for DateTime, DateTimeOffset and TimeSpan.
var itemType = type.GetItemType();
if (itemType is DateTimeDataViewType || itemType is DateTimeOffsetDataViewType || itemType is TimeSpanDataViewType)
data = new TypeConvertingTransformer(env, origStratCol, DataKind.Int64, origStratCol).Transform(data);

var localSeed = seed.HasValue ? seed : ((ISeededEnvironment)env).Seed.HasValue ? ((ISeededEnvironment)env).Seed : null;
var columnOptions =
localSeed.HasValue ?
new HashingEstimator.ColumnOptions(samplingKeyColumn, origStratCol, 30, (uint)localSeed.Value, combine: true) :
new HashingEstimator.ColumnOptions(samplingKeyColumn, origStratCol, 30, combine: true);

@harishsk harishsk Mar 3, 2020

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.

Similar comment as before. If the 30bits hashing is standard, can we define it as some global constant somewhere?

data = new HashingEstimator(env, columnOptions).Fit(data).Transform(data);
}
else
Expand Down
4 changes: 1 addition & 3 deletions src/Microsoft.ML.Data/Transforms/ConversionsExtensionsCatalog.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,7 @@ public static HashingEstimator Hash(this TransformsCatalog.ConversionTransforms
/// ]]></format>
/// </example>
public static HashingEstimator Hash(this TransformsCatalog.ConversionTransforms catalog, params ColumnOptions[] columns)
=> new HashingEstimator(CatalogUtils.GetEnvironment(catalog),
columns.Select(x => new ColumnOptionsInternal(x.Name, x.InputColumnName, x.NumberOfBits, x.Seed,
x.UseOrderedHashing, x.MaximumNumberOfInverts)).ToArray());
=> new HashingEstimator(CatalogUtils.GetEnvironment(catalog), columns);

/// <summary>
/// Create a <see cref="TypeConvertingEstimator"/>, which converts the type of the data to the type specified in <paramref name="outputKind"/>.
Expand Down
Loading