Skip to content

Add a DNN Image Featurizer Transform Estimator #1447

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

Merged
merged 56 commits into from
Nov 27, 2018
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
bc71575
Skeleton for implementing with a new tranformer
vaeksare Oct 26, 2018
32dc90c
Approach using a chain of transformers
vaeksare Oct 26, 2018
38fecff
Reworked to use estimator chain
vaeksare Oct 26, 2018
5e1faa5
Add basic testing
vaeksare Oct 27, 2018
3c3aa2a
Reworked to use extension methods
vaeksare Oct 29, 2018
66b621e
Create more extensive tests
vaeksare Oct 30, 2018
87a5d51
Remove changes to onnx transform tests
vaeksare Oct 30, 2018
5bec174
Rework to use CDN approach
vaeksare Oct 31, 2018
8876f02
Add static extensions
vaeksare Oct 31, 2018
2630f7b
Added test for static pipeline
vaeksare Nov 1, 2018
4a38bf9
Updated to use correct CDN location
vaeksare Nov 2, 2018
75b1b0d
Added some minor comments
vaeksare Nov 2, 2018
271ae03
Use extension methods and set up the model download/nuget placement
vaeksare Nov 7, 2018
2e5ef2d
Change the proj to try to fix download
vaeksare Nov 7, 2018
85ca884
Fix the download in csproj
vaeksare Nov 7, 2018
b76deda
Models successfully download into correct folder
vaeksare Nov 7, 2018
259ac05
Include the onnx models in the nuget package
vaeksare Nov 7, 2018
d093d4b
Fix tests and add documentation
vaeksare Nov 8, 2018
30b9488
Fixed building breaking due to merge
vaeksare Nov 8, 2018
a3792d9
Update the estimator name to match new name
vaeksare Nov 8, 2018
9338464
Change model download location and update the corresponding doc
vaeksare Nov 9, 2018
1e7c7ae
Actually fixed the download location
vaeksare Nov 9, 2018
ec352d4
Address PR comments and add experimental test (not enabled yet)
vaeksare Nov 9, 2018
ee24322
Changed documentation style to use xml
vaeksare Nov 9, 2018
6c97453
Add test that checks output results
vaeksare Nov 9, 2018
24caec9
Address more comments
vaeksare Nov 9, 2018
3975134
Change param name to be more accurate
vaeksare Nov 9, 2018
bf25371
Added refs to documentation
vaeksare Nov 9, 2018
c72bcfe
Add one more ref
vaeksare Nov 9, 2018
2bb2163
Added all 4 different models
vaeksare Nov 10, 2018
4cdd1eb
Address comments
vaeksare Nov 10, 2018
af7a844
Address more comments
vaeksare Nov 10, 2018
0e2d617
More comment fixes
vaeksare Nov 10, 2018
e9e60b0
Address some of the comments
vaeksare Nov 12, 2018
326d8bf
Replace TLC links
vaeksare Nov 12, 2018
579ad6b
Rework download location
vaeksare Nov 12, 2018
368cd64
Change model download to new redist proj
vaeksare Nov 14, 2018
1d106bd
Add the models to content folder
vaeksare Nov 14, 2018
1bebc64
Address some comments
vaeksare Nov 21, 2018
22e385b
Merge remote-tracking branch 'upstream/master' into dnn-image-feat
vaeksare Nov 21, 2018
8f9e808
Revert "Merge remote-tracking branch 'upstream/master' into dnn-image…
vaeksare Nov 21, 2018
a1e571d
Merge remote-tracking branch 'upstream/master' into dnn-image-feat
vaeksare Nov 21, 2018
55b5b0f
updated libmf ref
vaeksare Nov 21, 2018
7405fce
Updated to fix issues related to OnnxTransform changes
vaeksare Nov 22, 2018
ce93709
Merge remote-tracking branch 'upstream/master' into dnn-image-feat
vaeksare Nov 22, 2018
02e8ebc
Update to new namespace
vaeksare Nov 22, 2018
c519a19
Address more comments
vaeksare Nov 26, 2018
a687ccb
Reorganize model downloading
vaeksare Nov 26, 2018
52a3e6a
Fix test downloading
vaeksare Nov 26, 2018
9f566a2
Removed model duplication in NuGets
vaeksare Nov 26, 2018
d33eb06
Updated targets location
vaeksare Nov 26, 2018
3e4db44
Fixed issues with target content inclusion
vaeksare Nov 26, 2018
5fbd41d
Change targets file to be consistent
vaeksare Nov 27, 2018
bdc9f60
Fix bug due to targets
vaeksare Nov 27, 2018
8effaf9
Simplify model copy in tests
vaeksare Nov 27, 2018
b27ebe2
Address minor comment
vaeksare Nov 27, 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
11 changes: 11 additions & 0 deletions Microsoft.ML.sln
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.ML.SamplesUtils",
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.ML.Recommender", "src\Microsoft.ML.Recommender\Microsoft.ML.Recommender.csproj", "{C8E1772B-DFD9-4A4D-830D-6AAB1C668BB3}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.ML.DnnImageFeaturizer.ResNet18", "src\Microsoft.ML.DnnImageFeaturizer.ResNet18\Microsoft.ML.DnnImageFeaturizer.ResNet18.csproj", "{9222FC9D-599A-49A5-B685-08CC9A5C81D7}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -511,6 +513,14 @@ Global
{C8E1772B-DFD9-4A4D-830D-6AAB1C668BB3}.Release|Any CPU.Build.0 = Release|Any CPU
Copy link
Member

@wschin wschin Nov 10, 2018

Choose a reason for hiding this comment

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

Not aligned? #Resolved

{C8E1772B-DFD9-4A4D-830D-6AAB1C668BB3}.Release-Intrinsics|Any CPU.ActiveCfg = Release-Intrinsics|Any CPU
{C8E1772B-DFD9-4A4D-830D-6AAB1C668BB3}.Release-Intrinsics|Any CPU.Build.0 = Release-Intrinsics|Any CPU
{9222FC9D-599A-49A5-B685-08CC9A5C81D7}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{9222FC9D-599A-49A5-B685-08CC9A5C81D7}.Debug|Any CPU.Build.0 = Debug|Any CPU
{9222FC9D-599A-49A5-B685-08CC9A5C81D7}.Debug-Intrinsics|Any CPU.ActiveCfg = Debug-Intrinsics|Any CPU
{9222FC9D-599A-49A5-B685-08CC9A5C81D7}.Debug-Intrinsics|Any CPU.Build.0 = Debug-Intrinsics|Any CPU
{9222FC9D-599A-49A5-B685-08CC9A5C81D7}.Release|Any CPU.ActiveCfg = Release|Any CPU
{9222FC9D-599A-49A5-B685-08CC9A5C81D7}.Release|Any CPU.Build.0 = Release|Any CPU
{9222FC9D-599A-49A5-B685-08CC9A5C81D7}.Release-Intrinsics|Any CPU.ActiveCfg = Release-Intrinsics|Any CPU
{9222FC9D-599A-49A5-B685-08CC9A5C81D7}.Release-Intrinsics|Any CPU.Build.0 = Release-Intrinsics|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down Expand Up @@ -567,6 +577,7 @@ Global
{ECB71297-9DF1-48CE-B93A-CD969221F9B6} = {DA452A53-2E94-4433-B08C-041EDEC729E6}
Copy link
Member

@wschin wschin Nov 10, 2018

Choose a reason for hiding this comment

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

Not aligned? #Resolved

{11A5210E-2EA7-42F1-80DB-827762E9C781} = {09EADF06-BE25-4228-AB53-95AE3E15B530}
{C8E1772B-DFD9-4A4D-830D-6AAB1C668BB3} = {09EADF06-BE25-4228-AB53-95AE3E15B530}
{9222FC9D-599A-49A5-B685-08CC9A5C81D7} = {09EADF06-BE25-4228-AB53-95AE3E15B530}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {41165AF1-35BB-4832-A189-73060F82B01D}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<Project Sdk="Microsoft.NET.Sdk" DefaultTargets="Pack">

<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<PackageDescription>ML.NET component for pretrained ResNet18 image featurization</PackageDescription>
</PropertyGroup>

<ItemGroup>
<ProjectReference Include="../Microsoft.ML.OnnxTransform/Microsoft.ML.OnnxTransform.nupkgproj" />
</ItemGroup>

<ItemGroup>
<Content Include="$(LocalAppData)\mlnet-resources\ResNet18Onnx\ResNet18.onnx" Pack="true" PackagePath=".\lib\netstandard2.0\ResNet18Onnx" />
<Content Include="$(LocalAppData)\mlnet-resources\ResNetPrepOnnx\ResNetPreprocess.onnx" Pack="true" PackagePath=".\lib\netstandard2.0\ResNetPrepOnnx" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<Project DefaultTargets="Pack">

<Import Project="Microsoft.ML.DnnImageFeaturizer.ResNet18.nupkgproj" />

</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
<Project Sdk="Microsoft.NET.Sdk">

<UsingTask TaskName="DownloadFilesFromUrl" AssemblyFile="$(ToolsDir)Microsoft.DotNet.Build.Tasks.dll"/>

<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<IncludeInPackage>Microsoft.ML.DnnImageFeaturizer.ResNet18</IncludeInPackage>
</PropertyGroup>

<ItemGroup>
<ProjectReference Include="..\Microsoft.ML.OnnxTransform\Microsoft.ML.OnnxTransform.csproj" />
</ItemGroup>

<PropertyGroup>
<LocalAppDataDir>$(LocalAppData)</LocalAppDataDir>
<MlnetAppDataDir>$(LocalAppData)/mlnet-resources</MlnetAppDataDir>
</PropertyGroup>

<ItemGroup>
<PreprocessFile Include="$(MlnetAppDataDir)/ResNetPrepOnnx/ResNetPreprocess.onnx"
Url="https://express-tlcresources.azureedge.net/image/ResNetPrepOnnx/ResNetPreprocess.onnx "
DestinationFile="$(MlnetAppDataDir)/ResNetPrepOnnx/ResNetPreprocess.onnx"
DestinationDir="$(MlnetAppDataDir)/ResNetPrepOnnx" />
</ItemGroup>

<ItemGroup>
<ModelFile Include="$(MlnetAppDataDir)/ResNet18Onnx/ResNet18.onnx"
Url="https://express-tlcresources.azureedge.net/image/ResNet18Onnx/ResNet18.onnx"
DestinationFile="$(MlnetAppDataDir)/ResNet18Onnx/ResNet18.onnx"
DestinationDir="$(MlnetAppDataDir)/ResNet18Onnx" />
</ItemGroup>

<Target Name="DownloadPrepModels" Inputs="@(PreprocessFile)" Outputs="%(PreprocessFile.DestinationFile)">
<Message Importance="High" Text="Downloading external model files... %(PreprocessFile.DestinationFile)" />
<MakeDir Directories="$(MlnetAppDataDir)" />
<MakeDir Directories="$(LocalAppDataDir)" />
<MakeDir Directories="%(PreprocessFile.DestinationDir)" />
<DownloadFilesFromUrl Items="@(PreprocessFile)"/>
</Target>

<Target Name="DownloadMainModels"
DependsOnTargets="DownloadPrepModels"
AfterTargets="Build"
Inputs="@(ModelFile)" Outputs="%(ModelFile.DestinationFile)">
<Message Importance="High" Text="Downloading external model files... %(ModelFile.DestinationFile)" />
<MakeDir Directories="%(ModelFile.DestinationDir)" />
<DownloadFilesFromUrl Items="@(ModelFile)"/>
</Target>

</Project>
50 changes: 50 additions & 0 deletions src/Microsoft.ML.DnnImageFeaturizer.ResNet18/ResNet18.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using Microsoft.ML.Runtime.Data;
using System.IO;
using System.Reflection;

namespace Microsoft.ML.Transforms
{
// This is an extension method to be used with the DnnImageFeaturizerTransform in order to use a pretrained ResNet18 model.
// The NuGet containing this extension is also guaranteed to include the binary model file. Note that when building the project
// containing this extension method, the corresponding binary model will be downloaded from the CDN at
// https://express-tlcresources.azureedge.net/image/ResNetPrepOnnx/ResNetPreprocess.onnx and
// https://express-tlcresources.azureedge.net/image/ResNet18Onnx/ResNet18.onnx and placed into the local app directory
// folder under mlnet-resources.
public static class ResNet18Extension
{
// If including this through a NuGet, the location of the model will be the same as of this file. This looks for the model there.
// This should be the default way to use ResNet18 if importing the model from a NuGet.
public static EstimatorChain<OnnxTransform> ResNet18(this DnnImageModelSelector model)
{
var modelChain = new EstimatorChain<OnnxTransform>();
var tempCol = "onnxDnnPrep";
var execDir = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location);

var prepEstimator = new OnnxScoringEstimator(model.Env, Path.Combine(execDir, "ResNetPrepOnnx", "ResNetPreprocess.onnx"), model.Input, tempCol);
var mainEstimator = new OnnxScoringEstimator(model.Env, Path.Combine(execDir, "ResNet18Onnx", "ResNet18.onnx"), tempCol, model.Output);
modelChain = modelChain.Append(prepEstimator);
modelChain = modelChain.Append(mainEstimator);
return modelChain;
}

// This allows a custom model location to be specified. This is mainly useful for other code inside of ML.NET (such as tests),
Copy link
Contributor

@TomFinley TomFinley Nov 9, 2018

Choose a reason for hiding this comment

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

mainly useful for other code inside of ML.NET (such as tests) [](start = 72, length = 61)

Then it should not be part of the public API surface area, or if you really intended it to be used by people, you should talk about how it is useful to them, rather than talking about the internal structure of our tests. #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed the documentation to talk about how it is also useful to public API (as I do believe this can come in very handy in certain scenarios and provides a lot of extra flexibility).


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

// but could also be used if one wishes to change the directory of the model for whatever reason. Note that because Onnx models
// must be in a directory all by themsleves for the OnnxTransform to work, this method appends a ResNet18Onnx/ResNetPrepOnnx subdirectory
// to the passed in directory to prevent having to make that directory manually each time.
public static EstimatorChain<OnnxTransform> ResNet18(this DnnImageModelSelector model, string modelDir)
Copy link
Contributor

@TomFinley TomFinley Nov 9, 2018

Choose a reason for hiding this comment

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

model [](start = 88, length = 5)

Naming this model is not an appropriate choice, since model typically refers to the result of training. I believe elsewhere we've been naming this type of object a context... e.g., MLContext, TextLoaderContext, and so on. #Resolved

{
var modelChain = new EstimatorChain<OnnxTransform>();
var tempCol = "onnxDnnPrep";

var prepEstimator = new OnnxScoringEstimator(model.Env, Path.Combine(modelDir, "ResNetPrepOnnx", "ResNetPreprocess.onnx"), model.Input, tempCol);
var mainEstimator = new OnnxScoringEstimator(model.Env, Path.Combine(modelDir, "ResNet18Onnx", "ResNet18.onnx"), tempCol, model.Output);
modelChain = modelChain.Append(prepEstimator);
modelChain = modelChain.Append(mainEstimator);
return modelChain;
}
}
}
101 changes: 101 additions & 0 deletions src/Microsoft.ML.OnnxTransform/DnnImageFeaturizerTransform.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using Microsoft.ML.Core.Data;
using Microsoft.ML.Runtime;
using Microsoft.ML.Runtime.Data;
using Microsoft.ML.Runtime.Internal.Utilities;
using Microsoft.ML.StaticPipe;
using Microsoft.ML.StaticPipe.Runtime;
using System;
using System.Collections.Generic;

namespace Microsoft.ML.Transforms
{
// This is a helper class that is required to use the DnnImageFeaturizer estimator.
// Note that by default, it is not usable as it does not have any valid methods that return an EstimatorChain that
Copy link
Contributor

@TomFinley TomFinley Nov 9, 2018

Choose a reason for hiding this comment

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

EstimatorChain [](start = 99, length = 14)

When referring to specific types, please use <see tags so that people can see what you're talking about more easily. #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

The issue here is that the ResNet18 example is just one of the possible examples that I am talking about, and it is in a separate project that does not have to be included with this one. I don't believe I can cref it without adding a using statement to bring it into scope, which I don't want to do as it breaks the whole reason of it being in a separate project to begin with.


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

Copy link
Member

Choose a reason for hiding this comment

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

I feel we still should do reference. You refer some types because they are used either explicitly or implicitly.


In reply to: 232397881 [](ancestors = 232397881,232382991)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. I added refs to everything that's not the model project themselves.


In reply to: 232413934 [](ancestors = 232413934,232397881,232382991)

// is used by the DnnImageFeaturizeEstimator.
// In order to use this, at least one model project with the corresponding extension methods must by included.
// See Microsoft.ML.DNNImageFeaturizer.ResNet18 for an example.
public class DnnImageModelSelector
Copy link
Contributor

@TomFinley TomFinley Nov 9, 2018

Choose a reason for hiding this comment

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

public class DnnImageModelSelector [](start = 4, length = 34)

Sealed, I assume? #Resolved

{
public readonly IHostEnvironment Env;
Copy link
Member

@eerhardt eerhardt Nov 19, 2018

Choose a reason for hiding this comment

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

These should be properties, and not fields.

public IHostEnvironment Env { get; } #Resolved

public readonly string Input;
Copy link
Contributor

@TomFinley TomFinley Nov 9, 2018

Choose a reason for hiding this comment

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

So, why does this need to be public? In other static extensions we've always succeeded in hiding it? #Resolved

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's just that it needs to be seen from our assemblies, please see my work with internals visible to and [BestFriend], it will prove useful here.


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

Copy link
Member Author

@vaeksare vaeksare Nov 9, 2018

Choose a reason for hiding this comment

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

Thanks! Redesigned the method so these do not have to be public.


In reply to: 232385063 [](ancestors = 232385063,232382715)

public readonly string Output;

public DnnImageModelSelector(IHostEnvironment env, string input, string output)
{
Env = env;
Input = input;
Output = output;
}
}

// The Dnn Image Featurizer is just a wrapper around two OnnxTransforms with present pretrained DNN models.
// Note that because of this, it only works on Windows machines as that is a constraint of the OnnxTransform.
Copy link
Member

@wschin wschin Nov 9, 2018

Choose a reason for hiding this comment

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

Please make it

... #Resolved

public sealed class DnnImageFeaturizerEstimator : IEstimator<TransformerChain<OnnxTransform>>
Copy link
Member

@wschin wschin Nov 9, 2018

Choose a reason for hiding this comment

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

IEstimator [](start = 54, length = 10)

ITransformer? #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

No? This is an estimator?


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

Copy link
Member

Choose a reason for hiding this comment

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

What about this is specific to:

  1. Deep neural nets, vs any kind of NN?
  2. Images?

Couldn't this type have a more general name, so I could use it with audio, and a non-deep NN?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nothing in particular, although currently that is the only use for this transform based on the currently available models which makes the name more descriptive for users trying to figure out what it's for. But with more extensions in the future it could definitely be used for more things. Not sure how likely that is to happen however, and whether it is worth it to give false expectations.


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

{
private readonly EstimatorChain<OnnxTransform> _modelChain;

// The function passed in to this method is expected to be an extension method on the DnnImageModelSelector class
// that creates a chain of two OnnxTransforms with specific models included together with that extension method.
// For an example, see Microsoft.ML.DnnImageFeaturizer.ResNet18.
Copy link
Member

@wschin wschin Nov 9, 2018

Choose a reason for hiding this comment

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

... . Also please use to refer the actual object in code. #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't believe I can use cref here as that other file is not a part of this project, and I don't want to include it with a using statement it in this project as that breaks the whole purpose of it being separated in the first place.


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

public DnnImageFeaturizerEstimator(IHostEnvironment env, Func<DnnImageModelSelector, EstimatorChain<OnnxTransform>> model, string input, string output)
{
_modelChain = model(new DnnImageModelSelector(env, input, output));
}

public TransformerChain<OnnxTransform> Fit(IDataView input)
{
return _modelChain.Fit(input);
}
Copy link
Member

@wschin wschin Nov 9, 2018

Choose a reason for hiding this comment

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

Can OnnxTransform fit now? I assume that this module is built from pre-trained models only. #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

OnnxTransform is a trivial estimator. Which means that fit actually just checks the schema but does no real fitting. The reason this is here is to adhere to the standard interface. Note this is also how other non trainable estimators work (can see OnnxScoringEstimator for an example).


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

Copy link
Member

Choose a reason for hiding this comment

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

Can you add this note to this function?


In reply to: 232403204 [](ancestors = 232403204,232401935)

Copy link
Member

Choose a reason for hiding this comment

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

Also, do you mean that "OnnxScoringEstimator" is an estimator? Here is a part of OnnxTransform --- public sealed class OnnxTransform : ITransformer, ICanSaveModel


In reply to: 232407856 [](ancestors = 232407856,232403204,232401935)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!


In reply to: 232407856 [](ancestors = 232407856,232403204,232401935)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, sorry. The OnnxScoringEstimator which this is based on is a trivial estimator.


In reply to: 232408322 [](ancestors = 232408322,232407856,232403204,232401935)


public SchemaShape GetOutputSchema(SchemaShape inputSchema)
{
return _modelChain.GetOutputSchema(inputSchema);
}
}

public static class DnnImageFeaturizerStaticExtensions
{
Copy link
Member

@wschin wschin Nov 10, 2018

Choose a reason for hiding this comment

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

Please put it in another file. #ByDesign

private sealed class OutColumn : Vector<float>
{
public PipelineColumn Input { get; }

public OutColumn(Vector<float> input, Func<DnnImageModelSelector, EstimatorChain<OnnxTransform>> model)
: base(new Reconciler(model), input)
{
Input = input;
}
}

private sealed class Reconciler : EstimatorReconciler
{
private readonly Func<DnnImageModelSelector, EstimatorChain<OnnxTransform>> _model;

public Reconciler(Func<DnnImageModelSelector, EstimatorChain<OnnxTransform>> model)
{
_model = model;
}

public override IEstimator<ITransformer> Reconcile(IHostEnvironment env,
PipelineColumn[] toOutput,
IReadOnlyDictionary<PipelineColumn, string> inputNames,
IReadOnlyDictionary<PipelineColumn, string> outputNames,
IReadOnlyCollection<string> usedNames)
{
Contracts.Assert(toOutput.Length == 1);

Copy link
Member

@wschin wschin Nov 10, 2018

Choose a reason for hiding this comment

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

Do you need to check if inputNames[outCol.Input] and outputNames[outCol] exist? #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it is checked before the Reconciler is called. This is something no other static api extensions check.


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

var outCol = (OutColumn)toOutput[0];
return new DnnImageFeaturizerEstimator(env, _model, inputNames[outCol.Input], outputNames[outCol]);
}
}

public static Vector<float> DnnImageFeaturizer(this Vector<float> input, Func<DnnImageModelSelector, EstimatorChain<OnnxTransform>> model)
{
Contracts.CheckValue(input, nameof(input));
return new OutColumn(input, model);
}
}
}
Loading