Skip to content

Image transforms become Estimators #753

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
Zruty0 merged 20 commits into from
Aug 30, 2018
Merged

Image transforms become Estimators #753

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
70d7611
Converted ImageLoaderTransform to be an Estimator/Transformer pair
Aug 27, 2018
1eeda48
Temp commit
Aug 27, 2018
5e32910
Another temp checkin
Aug 27, 2018
f0469e1
WIP 3
Aug 27, 2018
d2073eb
Added image resizers
Aug 27, 2018
f05ec52
Pixel extractor
Aug 27, 2018
a03994b
Minor fix
Aug 27, 2018
ef5cb62
Merge remote-tracking branch 'upstream/master' into feature/image-est…
Aug 27, 2018
55b33af
Fixed build
Aug 27, 2018
ac46be9
PR comments
Aug 28, 2018
013bfa4
Merge
Aug 28, 2018
11aa1fc
Added grayscale transform
Aug 28, 2018
281e731
wip one to one
Aug 28, 2018
baa5c34
Finished the mockup of one-to-one transformer base class.
Aug 28, 2018
245c344
Converted to inherit from base class
Aug 28, 2018
e7dc348
Workout test
Aug 28, 2018
d8c0648
Minor changes
Aug 29, 2018
3e845fd
PR comments
Aug 30, 2018
fe5374f
Merge remote-tracking branch 'upstream/master' into feature/image-est…
Aug 30, 2018
e660eeb
Fixed build
Aug 30, 2018
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
16 changes: 8 additions & 8 deletions src/Microsoft.ML.Core/Data/IEstimator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,18 @@ public enum VectorKind

public readonly string Name;
public readonly VectorKind Kind;
public readonly DataKind ItemKind;
public readonly ColumnType ItemType;

@TomFinley TomFinley Aug 28, 2018 •
edited by Zruty0
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.

ColumnType ItemType [](start = 28, length = 19)

This is somewhat problematic for key columns. How do we represent the two distinct concepts of a key-columns of known and unknown counts? #Pending

@Zruty0 Zruty0 Aug 28, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well, ItemType is never supposed to be a key. For example, for a scalar key column, the correct representation is Kind = Scalar, ItemType = PrimitiveType.U4, IsKey = true.

I think I should enforce it in constructor even.

I don't think we should even have key types of unknown counts: it is already causing issues for Term transform, and I'm yet to see any benefit of this type.

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

@TomFinley TomFinley Aug 30, 2018

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 agree that keys of unknown size should go away, but are we going to do this in this PR?

That we're representing a key-type using a column type which is not a key-type at all, and whose only connection to it is that their DataKind happen to be the same, is rather odd and unfortunate. I hope we can imagine a better way here, though I'm not sure I see one right off the bat.

In reply to: 213156549 [](ancestors = 213156549,213151407)

@Zruty0 Zruty0 Aug 30, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We are also representing a vector type using a column type which is not a vector-type at all, and whose only connection to it is that their ItemType happen to be the same :)

In reply to: 214125274 [](ancestors = 214125274,213156549,213151407)

public readonly bool IsKey;
public readonly string[] MetadataKinds;

public Column(string name, VectorKind vecKind, DataKind itemKind, bool isKey, string[] metadataKinds = null)
public Column(string name, VectorKind vecKind, ColumnType itemType, bool isKey, string[] metadataKinds = null)
{
Contracts.CheckNonEmpty(name, nameof(name));
Contracts.CheckValueOrNull(metadataKinds);

Name = name;
Kind = vecKind;
ItemKind = itemKind;
ItemType = itemType;
IsKey = isKey;
MetadataKinds = metadataKinds ?? new string[0];
}
Expand All @@ -51,7 +51,7 @@ public Column(string name, VectorKind vecKind, DataKind itemKind, bool isKey, st
/// requirement.
///
/// Namely, it returns true iff:
/// - The <see cref="Name"/>, <see cref="Kind"/>, <see cref="ItemKind"/>, <see cref="IsKey"/> fields match.
/// - The <see cref="Name"/>, <see cref="Kind"/>, <see cref="ItemType"/>, <see cref="IsKey"/> fields match.
/// - The <see cref="MetadataKinds"/> of <paramref name="inputColumn"/> is a superset of our <see cref="MetadataKinds"/>.
/// </summary>
public bool IsCompatibleWith(Column inputColumn)
Expand All @@ -61,7 +61,7 @@ public bool IsCompatibleWith(Column inputColumn)
return false;
if (Kind != inputColumn.Kind)
return false;
if (ItemKind != inputColumn.ItemKind)
if (!ItemType.Equals(inputColumn.ItemType))
return false;
if (IsKey != inputColumn.IsKey)
return false;
Expand All @@ -72,7 +72,7 @@ public bool IsCompatibleWith(Column inputColumn)

public string GetTypeString()
{
string result = ItemKind.ToString();
string result = ItemType.ToString();
if (IsKey)
result = $"Key<{result}>";
if (Kind == VectorKind.Vector)
Expand Down Expand Up @@ -110,13 +110,13 @@ public static SchemaShape Create(ISchema schema)
else
vecKind = Column.VectorKind.Scalar;

var kind = type.ItemType.RawKind;
var itemKind = type.ItemType.RawKind;
var isKey = type.ItemType.IsKey;

var metadataNames = schema.GetMetadataTypes(iCol)
.Select(kvp => kvp.Key)
.ToArray();
cols.Add(new Column(schema.GetColumnName(iCol), vecKind, kind, isKey, metadataNames));
cols.Add(new Column(schema.GetColumnName(iCol), vecKind, PrimitiveType.FromKind(itemKind), isKey, metadataNames));
}
}
return new SchemaShape(cols.ToArray());
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.ML.ImageAnalytics/EntryPoints/ImageAnalytics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public static class ImageAnalytics
public static CommonOutputs.TransformOutput ImageLoader(IHostEnvironment env, ImageLoaderTransform.Arguments input)
{
var h = EntryPointUtils.CheckArgsAndCreateHost(env, "ImageLoaderTransform", input);
var xf = new ImageLoaderTransform(h, input, input.Data);
var xf = ImageLoaderTransform.Create(h, input, input.Data);
return new CommonOutputs.TransformOutput()
{
Model = new TransformModel(h, xf, input.Data),
Expand All @@ -29,7 +29,7 @@ public static CommonOutputs.TransformOutput ImageLoader(IHostEnvironment env, Im
public static CommonOutputs.TransformOutput ImageResizer(IHostEnvironment env, ImageResizerTransform.Arguments input)
{
var h = EntryPointUtils.CheckArgsAndCreateHost(env, "ImageResizerTransform", input);
var xf = new ImageResizerTransform(h, input, input.Data);
var xf = ImageResizerTransform.Create(h, input, input.Data);
return new CommonOutputs.TransformOutput()
{
Model = new TransformModel(h, xf, input.Data),
Expand Down
Loading