-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
[NNVM][TENSORFLOW] Mobilenet support. #1335
Conversation
srkreddy1238
commented
Jun 25, 2018
@tqchen @Huyuwei Please review. |
* reshape_like and Shape operators support. * fix in FusedBatchNorm * mobilenet frontend testcase updated.
# Rearrange inputs from | ||
# (data, gamma, beta, moving_mean, moving_variance) | ||
# to | ||
# (data, gamma, beta, moving_mean, moving_varience) |
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.
new_inputs is the same as inputs?
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.
Yes, FusedBatchNorm has same template as NNVM.
updated this unnecessary assignment.
nnvm/python/nnvm/testing/tf.py
Outdated
@@ -227,3 +227,31 @@ def get_workload_inception_v1(): | |||
graph_def.ParseFromString(f.read()) | |||
graph = tf.import_graph_def(graph_def, name='') | |||
return (image_data, tvm_data, graph_def) | |||
|
|||
def get_workload_mobilenet(): |
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.
get_workload_inception_v1() get_workload_inception_v3() get_workload_mobilenet()
these functions can be merged into a unified one that takes url and returns a tensorflow model
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.
Unified the model import across all these.
graph_def.ParseFromString(f.read()) | ||
graph = tf.import_graph_def(graph_def, name='') | ||
return graph_def | ||
|
||
def get_workload_inception_v3(): |
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.
get_workload_inception_v3() and get_workload_inception_v1() should also have only one line code as get_workload_mobilenet()
No need to prepare inpur data here, use random input for all tests
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.
InceptionV1 and V3 has some challenges using random data now.
-
Pool operators has difference in implementation with SAME padding. we use pad operator to meet 4 sides padding but the pad_value is 0 or passed value. In fact we should not consider the padded value in operation (avg/ max). At least maxpool I could workaround by pad_value = -(FLOAT_MAX) but avg pool is a tricky now.
-
Actual input for V1 is jpeg encoded stream. TF use decode_jpeg operator for this. NNVM we bypass it and feed decoded frame directly. I could try this by taking random_data, encoding and giving to tensorflow once the pool operators perform consistently.
I will revisit these test_cases once I get the pool ops working in sync with tensorflow.
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.
I see. Since now count_include_pad is introduced to AvgPool: #1163, can you use it to handle same padding?
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.
Great. In fact the pool operator test_cases I kept on hold on these reasons.
I will revisit the pool ops now. Thanks.
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.
Taking this discussion to https://discuss.tvm.ai/t/avgpool-same-padding/314 to resolve avg_pool issue.
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 can be merged now.