-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
ImageModels in tensorflow are 4 dimensional. #2872
Conversation
@@ -730,7 +730,7 @@ private sealed class Mapper : MapperBase | |||
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) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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]
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Codecov Report
@@ Coverage Diff @@
## master #2872 +/- ##
==========================================
+ Coverage 71.7% 71.7% +<.01%
==========================================
Files 811 811
Lines 142477 142477
Branches 16115 16115
==========================================
+ Hits 102162 102166 +4
+ Misses 35889 35886 -3
+ Partials 4426 4425 -1
|
Fixes #2778
Well, not exactly fixes, it's more like a hack.
Proper solution would be to implement Reshape transform #765