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.

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

TFLite SSD model with post processing. #247

Merged
merged 2 commits into from
May 6, 2020
Merged

Conversation

anijain2305
Copy link
Contributor

@anijain2305 anijain2305 commented Apr 24, 2020

@apivovarov @tqchen
I will add a test case in TVM for TFLite frontend. There is NMS support but not tested through TFLite frontend.

The instructions are exactly same as that of

#186

But with post processing enabled. Thanks @apivovarov for providing the instructions.

@apivovarov
Copy link
Contributor

Archive is 26 MB. Do you need all files from it? Can you shrink its size by removing unnecessary files?

@anijain2305
Copy link
Contributor Author

There is only file inside - Just TFlite model

@tqchen
Copy link
Member

tqchen commented Apr 24, 2020

is it possible to manually construct a tflite model that is smaller? e.g. do a graph cut to cut off the previous layers and only include the NMS part

@anijain2305
Copy link
Contributor Author

I don't have experience with TF graph transforms, but it might be possible.
At the same time, I think there is value in an e2e graph testing in TVM.
Actually the other model in this directory is the same SSD model but w/o NMS. So, this model is a superset. If this model can work, we can maybe remove the previous model.

Currently, there is accuracy mismatch so we cant replace the older model. I was hoping to add this model to open an Issue so that the original authors can take a look.

@apivovarov
Copy link
Contributor

yes, old file web-data/tensorflow/models/object_detection/ssd_mobilenet_v1_coco_2018_01_28_nopp.tgz (no post-processing) can be replaced with this new full model archive

@anijain2305
Copy link
Contributor Author

@tqchen apache/tvm#5439 is now passing, so I have removed the older TFLite file. Let me know if this is ok.

@anijain2305
Copy link
Contributor Author

Just realized that merging this will cause failure in TVM master as the current tests are dependent on the model (*_nopp.tgz) that I am removing in this PR.

I can send a separate TVM PR that just deletes the test using *_nopp.tgz

@tqchen Wanted to check if this makes sense?

@anijain2305
Copy link
Contributor Author

@tqchen Let me know if you need any more changes here. The TVM PR to remove old test is merged.

@anijain2305
Copy link
Contributor Author

Ping @tqchen :)

@tqchen tqchen merged commit ef0a0af into dmlc:master May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants