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

Jump to bottom

[Object Detection] Gluoncv SSD support on CPU #2353

Merged
merged 43 commits into from
Mar 11, 2019
Merged

[Object Detection] Gluoncv SSD support on CPU #2353

merged 43 commits into from
Mar 11, 2019

Conversation

kevinthesun
Copy link
Contributor

@kevinthesun kevinthesun commented Dec 29, 2018 •
edited
Loading

This PR refactors vision operators with hybrid script(thanks to @were). Now gluoncv SSD with resnet and mobilenet are supported in nnvm. SSD with vgg are not supported since x86 conv2d_NCHWc doesn't support dilation yet. Two tasks need to be done:

  • Multibox related ops are not supported by hybrid script yet since array container is not supported as argument. @were is working on this. Although gluoncv SSD models don't need these operators, legacy mxnet ssd models use them.
  • Support for relay ir.

@kevinthesun kevinthesun mentioned this pull request Jan 1, 2019
Closed
This was referenced Jan 11, 2019
Merged
Closed
@kevinthesun kevinthesun changed the title [WIP][Object Detection] Gluoncv SSD support on CPU [Object Detection] Gluoncv SSD support on CPU Jan 14, 2019
@kevinthesun
Copy link
Contributor Author

kevinthesun commented Jan 14, 2019 •
edited
Loading

Now full support for nnvm is ready. For relay, the only issue is flaky compilation probably caused by relay compiler engine: https://discuss.tvm.ai/t/relay-flaky-compilation-behavior/1478
ssd vgg is not supported yet, since dilation conv2d optimization is not supported in cpu.

The code is ready to be reviewed now. @tqchen @zhreshold @Laurawly @vinx13 @were

Thanks for @were 's support of hybrid script!

@kevinthesun kevinthesun force-pushed the RefactorSSDOperator branch 2 times, most recently from c1a4876 to fbe61e9 Compare January 15, 2019 00:44
vinx13
vinx13 requested changes Jan 15, 2019
View reviewed changes
tutorials/nnvm/deploy_ssd.py Outdated
@@ -4,22 +4,21 @@
**Author**: `Yao Wang <https://github.com/kevinthesun>`_

This article is an introductory tutorial to deploy SSD models with TVM.
<<<<<<< HEAD
Copy link
Member

Choose a reason for hiding this comment

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

unresolved conflict

@kevinthesun kevinthesun force-pushed the RefactorSSDOperator branch 2 times, most recently from 0b243b8 to 36250a6 Compare January 15, 2019 18:10
@kevinthesun
Copy link
Contributor Author

kevinthesun commented Jan 15, 2019 •
edited
Loading

The relay issue has been resolved.
@tqchen We might need gluoncv in ci.

Laurawly
Laurawly requested changes Jan 15, 2019
View reviewed changes
nnvm/tests/python/compiler/test_top_level4.py Outdated
np_out2[i, j, k] = -1

target = "llvm"
ctx = tvm.cpu()
Copy link
Contributor

Choose a reason for hiding this comment

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

Expanding target and ctx to for target, ctx in ctx_list(): can make this function look more generalized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will it fail in CI if gpu targets haven't been supported yet?

Copy link
Contributor

Choose a reason for hiding this comment

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

If so, adding if target == 'cuda' : continue will work.

topi/tests/python/test_topi_vision.py Outdated
@@ -46,7 +96,7 @@ def check_device(device):
f(tvm_data, tvm_valid_count, tvm_out)
tvm.testing.assert_allclose(tvm_out.asnumpy(), np_result, rtol=1e-4)

for device in ['llvm', 'opencl', 'cuda']:
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this one fail with 'opencl' and 'cuda'?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

get_valid_counts need to be implemented for gpu targets.

tutorials/nnvm/deploy_ssd.py Outdated
'ssd_512_mobilenet1_0_coco',
]

model_name = "ssd_512_resnet50_v1_voc"
dshape = (1, 3, 512, 512)
dtype = "float32"
target = "llvm"
ctx = tvm.cpu()
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe better to use ctx_list to make it more generalized.

@kevinthesun
Copy link
Contributor Author

@tqchen I added pip3 install gluoncv to Dockerfile.ci_gpu and Dockerfile.demo_gpu, but still can't import gluoncv. Is anywhere I missed?

@tqchen
Copy link
Member

tqchen commented Jan 18, 2019

To make sure we cache the image correctly without repetitive rebuilds. The docker image does not rebuild automatically, please send a separate PR to update the docker file, and I will update the docker image.

@kevinthesun kevinthesun force-pushed the RefactorSSDOperator branch 2 times, most recently from 81413f2 to 7eb53da Compare January 22, 2019 23:59
@kevinthesun
Copy link
Contributor Author

@tqchen Docker image not updated yet?

@tqchen
Copy link
Member

tqchen commented Jan 27, 2019

Working on the docker now.

@tqchen
Copy link
Member

tqchen commented Jan 28, 2019

Please check if the new ci error has things to do with this PR

@kevinthesun
Copy link
Contributor Author

I just ran this test 10000 times locally but didn't get any failure. I'll re-trigger the ci to see what's the problem.

@tqchen
Copy link
Member

tqchen commented Jan 28, 2019

The CI is now green, but please comment on the flaky case, to see if that could due to random-ness and tie (similar to argmax) or it is something that we can ignore

@kevinthesun kevinthesun force-pushed the RefactorSSDOperator branch from 99556f4 to d20024c Compare March 11, 2019 05:55
@tqchen tqchen merged commit d2f29ba into apache:master Mar 11, 2019
@tqchen tqchen added status: accepted and removed status: need update need update based on feedbacks labels Mar 11, 2019
@tqchen
Copy link
Member

tqchen commented Mar 11, 2019 •
edited
Loading

Thanks, @kevinthesun @were @vinx13 @Laurawly this is now merged

wweic pushed a commit to neo-ai/tvm that referenced this pull request Mar 12, 2019
@kevinthesun @wweic
[Object Detection] Gluoncv SSD support on CPU (apache#2353)
5aa6faa
@Laurawly Laurawly mentioned this pull request Mar 12, 2019
Merged
wweic pushed a commit to neo-ai/tvm that referenced this pull request Mar 12, 2019
@kevinthesun @wweic
[Object Detection] Gluoncv SSD support on CPU (apache#2353)
7726031
@kevinthesun kevinthesun deleted the RefactorSSDOperator branch May 28, 2019 23:20
@tqchen tqchen mentioned this pull request Nov 8, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Laurawly Laurawly Laurawly approved these changes

@vinx13 vinx13 vinx13 approved these changes

@tqchen tqchen tqchen approved these changes

@were were were approved these changes

Assignees

@tqchen tqchen

Labels
status: accepted
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@kevinthesun @tqchen @Laurawly @zhreshold @apivovarov @were @vinx13 @icemelon