Skip to content
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

ImageModels in tensorflow are 4 dimensional. #2872

Closed
wants to merge 1 commit into from
Closed

ImageModels in tensorflow are 4 dimensional. #2872

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/Microsoft.ML.TensorFlow/TensorflowTransform.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,7 @@ public Mapper(TensorFlowTransformer parent, DataViewSchema inputSchema) :
var originalShape = _parent.TFInputShapes[i];
var shape = originalShape.ToIntArray();

var colTypeDims = vecType.Dimensions.Select(dim => (long)dim).ToArray();
var colTypeDims = vecType.Dimensions.Prepend(1).Select(dim => (long)dim).ToArray();
if (shape == null)
Copy link
Member

Choose a reason for hiding this comment

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

This is not right. What if the expected shape is really 3-D? In addition, tensorflow internally can do reshape. Why can't we put the weight on tensorflow's shoulder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, it doesn't. At least for image models we use.
I'm actually not sure why, considering what usually models have variable tensors like [?,224,224,3], but in one which samples has problems they have [1,224,224,3]

Copy link
Member

@wschin wschin Mar 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Then we need to fix those models or edit them before their execution. It's not ideal to have an assumption which is true only for some image models.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Ivanidzo4ka, I agree with @wschin. It will create problems for models that don't have batch dimension. There were a couple of issues related to that on github. I already talked to @shauheen and @CESARDELATORRE on this and proposed following solution.

The ultimate solution is to have a reshape transform that user can use to reshape their data. However, the temporary solution would be to add a parameter in options class called “AddBatchDimensionOnInput”. When user set it to true, batch dimension would be added to the inputs otherwise not.

Can you instead do it for now in this PR? We can then discuss adding reshape transform later.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is the inception model from google.
https://storage.googleapis.com/download.tensorflow.org/models/inception5h.zip

Which takes input data of any shape. So we cannot infer shape from the input. There is a convolution layer just after the input which requires 4-D input and that's the problem.

_fullySpecifiedShapes[i] = new TFShape(colTypeDims);
else
Expand Down