Handle inputs with unknown shapes in TensorFlow #857

yaeldekel · 2018-09-07T21:56:30Z

This PR adds support for unknown shapes in the inputs and in the outputs of TensorFlow transform.
Closes #848 .

Ivanidzo4ka · 2018-09-08T00:46:23Z

            _host.CheckNonEmpty(output, nameof(outputs));

probably should be same as in input, can you change it to CheckNonWhiteSpace ? #Resolved

Refers to: src/Microsoft.ML.TensorFlow/TensorflowTransform.cs:210 in 34f91f8. [](commit_id = 34f91f8, deletion_comment = False)

Ivanidzo4ka · 2018-09-08T00:46:33Z

src/Microsoft.ML.TensorFlow/TensorflowTransform.cs

@@ -191,6 +191,8 @@ private TensorFlowTransform(IHostEnvironment env, byte[] modelBytes, string[] in
            Contracts.CheckValue(env, nameof(env));
            _host = env.Register(nameof(RegistrationName));
            _host.CheckValue(modelBytes, nameof(modelBytes));
+            _host.CheckNonEmpty(inputs, nameof(inputs));


inputs [](start = 47, length = 6)

outputs as well? #Resolved

Ivanidzo4ka · 2018-09-08T00:47:50Z

src/Microsoft.ML.TensorFlow/TensorflowTransform.cs

-                resultDic[Transformer.Outputs[i]] = new SchemaShape.Column(Transformer.Outputs[i], SchemaShape.Column.VectorKind.Vector, Transformer.OutputTypes[i].ItemType, false);
+            {
+                resultDic[Transformer.Outputs[i]] = new SchemaShape.Column(Transformer.Outputs[i],
+                    Transformer.OutputTypes[i].VectorSize > 0 ? SchemaShape.Column.VectorKind.Vector


.VectorSize > 0 [](start = 46, length = 15)

can we use IsKnownSizeVector? #Resolved

Ivanidzo4ka · 2018-09-08T01:03:49Z

src/Microsoft.ML.TensorFlow/TensorflowTransform.cs

@@ -311,26 +319,56 @@ public Mapper(IHostEnvironment env, TensorFlowTransform parent, ISchema inputSch
                _schema = inputSchema;
                _inputColIndices = new int[_parent.Inputs.Length];
                _isInputVector = new bool[_parent.Inputs.Length];
+                _fullySpecifiedShapes = new TFShape[_parent.Inputs.Length];


_fullySpecifiedShapes [](start = 16, length = 21)

I feel like this one should be part of Transformer, rather than mapper.
You do estimator.Fit(somedata) and gain transformer, and it resolves it's variable lengths[?,?,3] as [3,3,3].
Not sure it would be right to accept data rather than [3,3,3] to transformer after that.

(Same probably states for _isInputVector, not sure why I didn't put it to Transformer)
@Zruty0 to make sure I correctly understand estimator/transformer business.
#Closed

The problem is that when you instantiate the estimator you only have the TF model and not the IDV, so we don't necessarily know all the dimensions in the shape. However, in order to convert a VBuffer to a Tensor we need to know the fully specified shape.

Instead of having this field, I could instantiate a new shape object on every getter call, using _inputColIndices and _schema to figure out the input size. Do you think this is a good solution?

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

I think I just have incorrect understanding of estimator/transformer, and we don't have to have same schema for input fitting and input transforming.

In reply to: 216382814 [](ancestors = 216382814,216115594)

zeahmed · 2018-09-10T17:20:20Z

src/Microsoft.ML.TensorFlow/TensorflowTransform.cs

@@ -233,7 +240,7 @@ private TensorFlowTransform(IHostEnvironment env, byte[] modelBytes, string[] in
            {
                var tfOutput = new TFOutput(Graph[Outputs[i]]);
                var shape = Graph.GetTensorShape(tfOutput);
-                int[] dims = shape.ToIntArray().Skip(shape[0] == -1 ? BatchSize : 0).ToArray();
+                int[] dims = shape.NumDimensions > 0 ? shape.ToIntArray().Skip(shape[0] == -1 ? BatchSize : 0).ToArray() : new[] { 0 };


0 [](start = 131, length = 1)

Does this zero mean variable length? #Closed

Yes.

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

zeahmed

abgoswam · 2018-09-10T20:13:59Z

test/Microsoft.ML.Tests/ScenariosWithDirectInstantiation/TensorflowTests.cs

+                        getNum(ref buffer);
+                        getClasses(ref buffer);
+                    }
+                }


do we want to validate anything here ? #Closed

The images I am using here don't have any detections, so all of the outputs will be all 0. Once I get the models uploaded, I will also upload some images that have detections, then I can add validation of the outputs.

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

abgoswam · 2018-09-10T20:20:56Z

test/Microsoft.ML.Tests/ScenariosWithDirectInstantiation/TensorflowTests.cs

+            {
+                ModelFile = model_location,
+                OutputColumns = new[] { "Softmax", "dense/Relu" },
+                InputColumns = new[] { "Placeholder", "reshape_input" }


reshape_input [](start = 55, length = 13)

this is specified as an input, but I do not see it when passing in the data. Is this required ?

It is required, this column is created in line 273 by the CopyColumns transform.
This input is required for computing the "dense/Relu" output. Actually, I am not sure why it is needed, since if I understand correctly, reshape_input is computed by the input layer Placeholder by simply reshaping from 28x28 to 784. @zeahmed , do you know why "Placeholder" is not enough to compute "dense/Relu"?

This is the problem with the model. https://github.com/tensorflow/models/blob/master/official/mnist/mnist.py

If you want to access the features (named dense/Relu) only then reshape_input is required if you want to access the Softmax then Placeholder is required. This is the problem with model. If you closely look at the model graph attached. You would observe two graph that are working in parallel. Having said that I think its a good model to test two inputs and two outputs.

If you think if its going to make an issue I can update the model.

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

Also, i think we should rename the nodes properly to avoid strange names like dense/Relu etc....:)

In reply to: 216502475 [](ancestors = 216502475,216485826)

abgoswam · 2018-09-10T20:26:40Z

src/Microsoft.ML.TensorFlow/TensorflowTransform.cs

-                for (int j = 0; j < TFInputShapes[i].NumDimensions; j++)
-                    newShape[j] = TFInputShapes[i][j] == -1 ? BatchSize : TFInputShapes[i][j];
-                TFInputShapes[i] = new TFShape(newShape);
+                if (TFInputShapes[i].NumDimensions != -1)


if (TFInputShapes[i].NumDimensions != -1) [](start = 16, length = 41)

am curious - why did we need this check ?

Did some of the pre-trained models have TFInputShapes[i].NumDimensions == -1 (that we were not handling before)
#Resolved

If the shape is completely unknown, then its NumDimensions property is -1.

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

makes sense. in another comment i was asking if TF has this documented somewhere -- or we are using this as a heuristic based on models we have played with so far ?

In reply to: 216475056 [](ancestors = 216475056,216460739)

I haven't seen it documented, I just saw it by debugging different models.

In reply to: 216476239 [](ancestors = 216476239,216475056,216460739)

abgoswam · 2018-09-10T20:30:03Z

src/Microsoft.ML.TensorFlow/TensorflowTransform.cs

+                if (TFInputShapes[i].NumDimensions != -1)
+                {
+                    var newShape = new long[TFInputShapes[i].NumDimensions];
+                    newShape[0] = TFInputShapes[i][0] == -1 ? BatchSize : TFInputShapes[i][0];


newShape[0] = TFInputShapes[i][0] == -1 ? BatchSize : TFInputShapes[i][0]; [](start = 20, length = 74)

so we will have special handling only for the 1st dimension, and not for the other dimensions -- is that the intent ?

(looks like we should have been doing this previously too, instead of doing special handling for all the columns) #Resolved

Yes. This is because when the first dimension is -1 it indicates that the first dimension is the batch size. For any other dimension, it just means the dimension can be anything. For example, if the dimension is [?,?,?,3], then the first ? is for the batch size, and the other two are for the width and height of the image. In this case we don't want to change this to 1, we want to keep it as -1, so that we still need to fill in this value when we see the actual example and know its size.

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

yeap. good catch!

In reply to: 216477499 [](ancestors = 216477499,216461730)

abgoswam · 2018-09-10T20:33:34Z

src/Microsoft.ML.TensorFlow/TensorflowTransform.cs

@@ -233,7 +241,7 @@ private TensorFlowTransform(IHostEnvironment env, byte[] modelBytes, string[] in
            {
                var tfOutput = new TFOutput(Graph[Outputs[i]]);
                var shape = Graph.GetTensorShape(tfOutput);
-                int[] dims = shape.ToIntArray().Skip(shape[0] == -1 ? BatchSize : 0).ToArray();
+                int[] dims = shape.NumDimensions > 0 ? shape.ToIntArray().Skip(shape[0] == -1 ? BatchSize : 0).ToArray() : new[] { 0 };


shape.NumDimensions > 0 [](start = 29, length = 23)

I am presuming we are using this here as a check for models producing variable length outputs...Am i right ?

Does TF document such behaviour somewhere ? #Resolved

If the shape is unknown, it has shape.NumDimensions == -1, and shape.ToIntArray() == null, which would cause a null reference exception when we try to access shape[0].

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

abgoswam · 2018-09-10T20:45:47Z

src/Microsoft.ML.TensorFlow/TensorFlow/TensorGeneric.tt

@@ -37,13 +37,14 @@ namespace Microsoft.ML.Transforms.TensorFlow
        /// </summary>
        /// <typeparam name="T[]">.NET type of tensor to create</typeparam>
        /// <param name="data">value of tensor</param>
+        /// <param name="count">The number of elements in the tensor</param>


did we have to re-generate the TensorGeneric.cs file after making these changes ? #Resolved

Yes. Apparently TensorGeneric.tt regenerates TensorGeneric.cs automatically whenever it is saved.

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

awesome. thanks for the info.

In reply to: 216479096 [](ancestors = 216479096,216466572)

abgoswam · 2018-09-10T20:54:19Z

src/Microsoft.ML.TensorFlow/TensorflowTransform.cs

                for (int i = 0; i < _parent.Outputs.Length; i++)
                {
                    if (activeOutput(i))
                    {
                        var type = TFTensor.TypeFromTensorType(_parent.TFOutputTypes[i]);
                        _host.Assert(type == _parent.OutputTypes[i].ItemType.RawType);
                        var srcTensorGetters = GetTensorValueGetters(input);
-                        valueGetters.Add(Utils.MarshalInvoke(MakeGetter<int>, type, input, i, srcTensorGetters, activeOutputColNames, outputCache));
+                        valueGetters[i] = Utils.MarshalInvoke(MakeGetter<int>, type, input, i, srcTensorGetters, activeOutputColNames, outputCache);


valueGetters[i] [](start = 24, length = 15)

am curious about this bug in the getter -- could you kindly elaborate a bit on this ? .. is this some artifact of the activateOutput() call above.. #Resolved

This bug surfaced when I added the new unit test. It happens in the following situation:

Create a TensorFlowTransform that computes two outputs, say A and B.

Create another transform/learner that only uses B as input.

When we try to cursor over the data, the activeOutput predicate will say that output 0 is not active and output 1 is active, thus returning an array of length 1.

When we try to get the getter of column B, we do so using its index, which is 1 (the index of column A is 0 and the index of column B is 1). So we try to access the getters array which is of length 1, at index 1 which is out of bounds...

The fix was to always create an array with length equal to the number of output columns in the transform, but populate just the indices where the active columns are.

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

Ivanidzo4ka

…o we don't need a separate nuget for it to work.

abgoswam

Ivanidzo4ka · 2018-09-11T00:40:31Z

src/Microsoft.ML.TensorFlow/TensorFlow/Tensor.cs

        {
-            return SetupTensor(dt, dims, data, start: 0, count: data.Length, size: size);
+            return SetupTensor(dt, dims, data, start: 0, count: count, size: size);


start: 0, count: count, size: size [](start = 47, length = 34)

nit: do you need to specify param names here? don't you invoke function with all params already

yaeldMS added 4 commits September 6, 2018 15:01

Enable scoring Inception model and SSD model

ec5b2c1

Add a unit test for pipeline API.

36510f9

Merge from master.

de6b006

Update after merge with master

34f91f8

yaeldekel requested review from Ivanidzo4ka, abgoswam, zeahmed and Zruty0 September 7, 2018 21:56

Ivanidzo4ka reviewed Sep 8, 2018

View reviewed changes

Address PR comments

a9fde84

zeahmed reviewed Sep 10, 2018

View reviewed changes

Add a unit test, and fix a bug in the 'getter' creation method.

1874b50

zeahmed approved these changes Sep 10, 2018

View reviewed changes

abgoswam reviewed Sep 10, 2018

View reviewed changes

Ivanidzo4ka approved these changes Sep 10, 2018

View reviewed changes

Change new unit test to use LogisticRegression instead of LightGBM, s…

81bc481

…o we don't need a separate nuget for it to work.

abgoswam approved these changes Sep 10, 2018

View reviewed changes

Ivanidzo4ka reviewed Sep 11, 2018

View reviewed changes

Address PR comment.

a4241ff

yaeldekel merged commit 5666dd1 into dotnet:master Sep 11, 2018

yaeldekel deleted the unknownshape branch September 11, 2018 17:03

CESARDELATORRE mentioned this pull request Sep 12, 2018

Sample TensorFlow Scoring Inception v3 model using LearningPipeline dotnet/machinelearning-samples#42

Merged

ghost locked as resolved and limited conversation to collaborators Mar 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle inputs with unknown shapes in TensorFlow #857

Handle inputs with unknown shapes in TensorFlow #857

yaeldekel commented Sep 7, 2018

Ivanidzo4ka commented Sep 8, 2018 •

edited by yaeldekel

Loading

Ivanidzo4ka Sep 8, 2018 •

edited by yaeldekel

Loading

Ivanidzo4ka Sep 8, 2018 •

edited by yaeldekel

Loading

Ivanidzo4ka Sep 8, 2018 •

edited

Loading

yaeldekel Sep 10, 2018

Ivanidzo4ka Sep 10, 2018

zeahmed Sep 10, 2018 •

edited

Loading

yaeldekel Sep 10, 2018

zeahmed left a comment

abgoswam Sep 10, 2018 •

edited

Loading

yaeldekel Sep 10, 2018

abgoswam Sep 10, 2018

yaeldekel Sep 10, 2018

zeahmed Sep 10, 2018 •

edited

Loading

zeahmed Sep 10, 2018

abgoswam Sep 10, 2018 •

edited

Loading

yaeldekel Sep 10, 2018

abgoswam Sep 10, 2018

yaeldekel Sep 10, 2018

abgoswam Sep 10, 2018 •

edited

Loading

yaeldekel Sep 10, 2018

abgoswam Sep 10, 2018

abgoswam Sep 10, 2018 •

edited

Loading

yaeldekel Sep 10, 2018

abgoswam Sep 10, 2018 •

edited

Loading

yaeldekel Sep 10, 2018

abgoswam Sep 10, 2018

abgoswam Sep 10, 2018 •

edited

Loading

yaeldekel Sep 10, 2018

Ivanidzo4ka left a comment

abgoswam left a comment

Ivanidzo4ka Sep 11, 2018

Handle inputs with unknown shapes in TensorFlow #857

Handle inputs with unknown shapes in TensorFlow #857

Conversation

yaeldekel commented Sep 7, 2018

Ivanidzo4ka commented Sep 8, 2018 • edited by yaeldekel Loading

Ivanidzo4ka Sep 8, 2018 • edited by yaeldekel Loading

Choose a reason for hiding this comment

Ivanidzo4ka Sep 8, 2018 • edited by yaeldekel Loading

Choose a reason for hiding this comment

Ivanidzo4ka Sep 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zeahmed Sep 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zeahmed left a comment

Choose a reason for hiding this comment

abgoswam Sep 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zeahmed Sep 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abgoswam Sep 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abgoswam Sep 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abgoswam Sep 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abgoswam Sep 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abgoswam Sep 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ivanidzo4ka left a comment

Choose a reason for hiding this comment

abgoswam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ivanidzo4ka commented Sep 8, 2018 •

edited by yaeldekel

Loading

Ivanidzo4ka Sep 8, 2018 •

edited by yaeldekel

Loading

Ivanidzo4ka Sep 8, 2018 •

edited by yaeldekel

Loading

Ivanidzo4ka Sep 8, 2018 •

edited

Loading

zeahmed Sep 10, 2018 •

edited

Loading

abgoswam Sep 10, 2018 •

edited

Loading

zeahmed Sep 10, 2018 •

edited

Loading

abgoswam Sep 10, 2018 •

edited

Loading

abgoswam Sep 10, 2018 •

edited

Loading

abgoswam Sep 10, 2018 •

edited

Loading

abgoswam Sep 10, 2018 •

edited

Loading

abgoswam Sep 10, 2018 •

edited

Loading