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

Support for img norm in graph during onnx export #1027

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

nick-clarifai
Copy link

@nick-clarifai nick-clarifai commented Nov 9, 2021

Motivation

The purpose of this PR is to provide an option that does the image norm step of inference within the graph itself during the export to an ONNX model.

Modification

mmseg/datasets/pipelines/transforms.py:

  • an init flag is added which causes the image norm to be skipped during __call__ and causes the flag to be passed along in the img_norm_cfg

mmseg/models/segmentors/base.py

  • forward modified to do image normalization to the input if img_norm_cfg['normalize_in_graph'] is True

tools/pytorch2onnx.py:

  • command line arg added
  • normalize step in test pipeline is modified to skip normalization but still add img_norm_cfg
  • input is cloned in verification step so the pre-image norm input can be compared

Use cases

This would allow one to have a totally contained model that doesn't require extra python code for inference so that the ONNX model could easily be used in a setting such as NVIDIA Triton.

@CLAassistant
Copy link

CLAassistant commented Nov 9, 2021

CLA assistant check
All committers have signed the CLA.

@Junjun2016 Junjun2016 requested review from MengzhangLI and removed request for MengzhangLI November 10, 2021 05:27
@codecov
Copy link

codecov bot commented Nov 10, 2021

Codecov Report

Attention: Patch coverage is 27.27273% with 8 lines in your changes missing coverage. Please review.

Project coverage is 89.98%. Comparing base (9975c67) to head (f62cf69).
Report is 257 commits behind head on master.

Files with missing lines Patch % Lines
mmseg/models/segmentors/base.py 0.00% 6 Missing and 1 partial ⚠️
mmseg/datasets/pipelines/transforms.py 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1027      +/-   ##
==========================================
- Coverage   90.11%   89.98%   -0.14%     
==========================================
  Files         125      125              
  Lines        7262     7277      +15     
  Branches     1206     1210       +4     
==========================================
+ Hits         6544     6548       +4     
- Misses        515      524       +9     
- Partials      203      205       +2     
Flag Coverage Δ
unittests 89.98% <27.27%> (-0.14%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Junjun2016
Copy link
Collaborator

Hi @nick-clarifai
Thanks for your contribution.
We will review it in the near future.

@RockeyCoss
Copy link
Contributor

Please merge the master branch into your branch, thank you.

@Junjun2016
Copy link
Collaborator

Hi @RunningLeon
Please review it.

@RunningLeon
Copy link
Collaborator

@nick-clarifai Hi, thanks for your PR. If you add --normalize-in-graph in pytorch2onnx.py, then you might need to support it in downstream tool onnx2tensorrt.py as well.

@nick-clarifai
Copy link
Author

nick-clarifai commented Nov 18, 2021

@nick-clarifai Hi, thanks for your PR. If you add --normalize-in-graph in pytorch2onnx.py, then you might need to support it in downstream tool onnx2tensorrt.py as well.

Yeah this makes sense. The changes I can see being necessary would be allowing image normalization in the preprocessing step to be skipped and adding a command line arg to skip normalization? I'm happy to make these changes if we're on the same page. Let me know if there's something else I'm forgetting there.

@RunningLeon
Copy link
Collaborator

@nick-clarifai Hi, thanks for your PR. If you add --normalize-in-graph in pytorch2onnx.py, then you might need to support it in downstream tool onnx2tensorrt.py as well.

Yeah this makes sense. The changes I can see being necessary would be allowing image normalization in the preprocessing step to be skipped and adding a command line arg to skip normalization? I'm happy to make these changes if we're on the same page. Let me know if there's something else I'm forgetting there.

Yes. --normalize-in-graph should be added to onnx2tensorrt.py as well.

@nick-clarifai
Copy link
Author

Thanks for the feedback. onnx2tensorrt.py now has an option to skip image normalization in the preprocessing step. I didn't think it made sense to name the arg --normalize-in-graph because the tensorrt model will only do so if the input onnx model does normalize in graph, so I named the arg --skip-normalize instead. Let me know if you suggest any more changes.

@RunningLeon
Copy link
Collaborator

Thanks for the feedback. onnx2tensorrt.py now has an option to skip image normalization in the preprocessing step. I didn't think it made sense to name the arg --normalize-in-graph because the tensorrt model will only do so if the input onnx model does normalize in graph, so I named the arg --skip-normalize instead. Let me know if you suggest any more changes.

Actually, skip-normalize works for me.

@nick-clarifai
Copy link
Author

Is there something else that should be done before this could be merged? Let me know if there's anything I can do.

@RunningLeon
Copy link
Collaborator

Is there something else that should be done before this could be merged? Let me know if there's anything I can do.

@nick-clarifai Hi, sorry for the late reply. After resolve the rest comments, this PR is ready to go merge for me.

@nick-clarifai
Copy link
Author

Is there something else that should be done before this could be merged? Let me know if there's anything I can do.

@nick-clarifai Hi, sorry for the late reply. After resolve the rest comments, this PR is ready to go merge for me.

Thanks for the feedback @RunningLeon. I've made the requested changes.

Copy link
Collaborator

@RunningLeon RunningLeon left a comment

Choose a reason for hiding this comment

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

LGTM

@MengzhangLI
Copy link
Contributor

Hi, @nick-clarifai nick, sorry for late reply.

This pr is closely to merge, could you please fix unit test problem? Your modification is not coveraged by unit test.

image

Best,

@MengzhangLI MengzhangLI added WIP Work in process Merging PR waited for merging labels Jan 16, 2022
@nick-clarifai
Copy link
Author

Hi, @nick-clarifai nick, sorry for late reply.

This pr is closely to merge, could you please fix unit test problem? Your modification is not coveraged by unit test.

image

Best,

Sure, can you help me decide which is the best test to write? Should I write a test that runs the code from pytorch2onnx.py with the verification enabled? Or should I do something like add a test to tests/test_models/test_segmentors/test_encoder_decoder.py that makes sure the added code in mmseg/models/segmentors/base.py is run?

aravind-h-v pushed a commit to aravind-h-v/mmsegmentation that referenced this pull request Mar 27, 2023
@OpenMMLab-Assistant005
Copy link

Hi @nick-clarifai !We are grateful for your efforts in helping improve this open-source project during your personal time.
Welcome to join OpenMMLab Special Interest Group (SIG) private channel on Discord, where you can share your experiences, ideas, and build connections with like-minded peers. To join the SIG channel, simply message moderator— OpenMMLab on Discord or briefly share your open-source contributions in the #introductions channel and we will assist you. Look forward to seeing you there! Join us :https://discord.gg/UjgXkPWNqA
If you have a WeChat account,welcome to join our community on WeChat. You can add our assistant :openmmlabwx. Please add "mmsig + Github ID" as a remark when adding friends:)
Thank you again for your contribution❤

wjkim81 pushed a commit to wjkim81/mmsegmentation that referenced this pull request Dec 3, 2023
* map location to cpu when load checkpoint (open-mmlab#1007)

* [Enhancement] Support minus output feature index in mobilenet_v3 (open-mmlab#1005)

* fix typo in mobilenet_v3

* fix typo in mobilenet_v3

* use -1 to indicate output tensors from final stage

* support negative out_indices

* [Enhancement] inference speed and flops tools. (open-mmlab#986)

* add the function to test the dummy forward speed of models.

* add tools to test the flops and inference speed of multiple models.

* [Fix] Update pose tracking demo to be compatible with latest mmtrakcing (open-mmlab#1014)

* update mmtracking demo

* support both track_bboxes and track_results

* add docstring

* [Fix] fix skeleton_info of coco wholebody dataset (open-mmlab#1010)

* fix wholebody base dataset

* fix lint

* fix lint

Co-authored-by: ly015 <liyining0712@gmail.com>

* [Feature] Add ViPNAS models for wholebody keypoint detection (open-mmlab#1009)

* add configs

* add dark configs

* add checkpoint and readme

* update webcam demo

* fix model path in webcam demo

* fix unittest

* [Fix] Fix bbox label visualization (open-mmlab#1020)

* update model metafiles (open-mmlab#1001)

* update hourglass ae .md (open-mmlab#1027)

* [Feature] Add ViPNAS mbv3 (open-mmlab#1025)

* add vipnas mbv3

* test other variants

* submission for mmpose

* add unittest

* add readme

* update .yml

* fix lint

* rebase

* fix pytest

Co-authored-by: jin-s13 <jinsheng13@foxmail.com>

* [Enhancement] Set a random seed when the user does not set a seed (open-mmlab#1030)

* fix randseed

* fix lint

* fix import

* fix isort

* update yapf hook

* revert yapf version

* add cfg file for flops and speed test,  change the bulid_posenet to init_pose_model and fix some typo in cfg (open-mmlab#1028)

* [Enhancement] Add more functions for speed test tool (open-mmlab#1034)

* add batch size and device args in speed test script, and remove MMDataParallel warper

* add vipnas_mbv3 model

* fix dead link (open-mmlab#1038)

* Skip CI when some specific files were changed (open-mmlab#1041)

* update sigmas (open-mmlab#1040)

* add more configs, ckpts and logs for HRNet on PoseTrack18 (open-mmlab#1035)

* [Feature] Add PoseWarper dataset (open-mmlab#1006)

* add PoseWarper dataset and base class

* modify pipelines related to video

* add unittest for PoseWarper dataset

* add unittest for evaluation function in posetrack18-realted dataset, and add some annotations json files

* fix typo

* fix unittest CI failure

* fix typo

* add PoseWarper dataset and base class

* modify pipelines related to video

* add unittest for PoseWarper dataset

* add unittest for evaluation function in posetrack18-realted dataset, and add some annotations json files

* fix typo

* fix unittest CI failure

* fix typo

* modify some methods in the base class to improve code coverage rate

* recover some mistakenly-deleted notes

* remove test_dataset_info part for the new TopDownPoseTrack18VideoDataset class

* cancel uncompleted previous runs (open-mmlab#1053)

* [Doc] Add inference speed results (open-mmlab#1044)

* add docs related to inference speed results

* add corresponding Chinese docs and fix some typos

* add Chinese docs in readthedocs

* remove the massive table in readme

* minor modification to wording

Co-authored-by: ly015 <liyining0712@gmail.com>

* [Feature] Add PoseWarper detector model (open-mmlab#932)

* Add top down video detector module

* Add PoseWarper neck

* add function _freeze_stages

* fix typo

* modify PoseWarper detector and PoseWarperNeck

* fix typo

* modify posewarper detector and neck

* Delete top_down_video.py

change the base class of `PoseWarper` detector from `TopDownVideo` to `TopDown`

* fix spell typo

* modify detector and neck

* add unittest for detector and neck

* modify unittest for posewarper forward

* Add top down video detector module

* Add PoseWarper neck

* add function _freeze_stages

* fix typo

* modify PoseWarper detector and PoseWarperNeck

* fix typo

* modify posewarper detector and neck

* Delete top_down_video.py

change the base class of `PoseWarper` detector from `TopDownVideo` to `TopDown`

* fix spell typo

* modify detector and neck

* add unittest for detector and neck

* modify unittest for posewarper forward

* modify dependency on mmcv version in posewarper neck

* reduce memory cost in test

* modify flops tool for more flexible input format

* Add top down video detector module

* Add PoseWarper neck

* add function _freeze_stages

* fix typo

* modify PoseWarper detector and PoseWarperNeck

* fix typo

* modify posewarper detector and neck

* Delete top_down_video.py

change the base class of `PoseWarper` detector from `TopDownVideo` to `TopDown`

* fix spell typo

* modify detector and neck

* add unittest for detector and neck

* modify unittest for posewarper forward

* Add PoseWarper neck

* modify PoseWarper detector and PoseWarperNeck

* modify posewarper detector and neck

* Delete top_down_video.py

change the base class of `PoseWarper` detector from `TopDownVideo` to `TopDown`

* fix spell typo

* modify detector and neck

* add unittest for detector and neck

* modify unittest for posewarper forward

* modify dependency on mmcv version in posewarper neck

* reduce memory cost in test

* modify flops tool for more flexible input format

* modify the posewarper detector description

* modify some arguments and related fields

* modify default values for some args

* fix readthedoc bulid typo

* fix ignore path (open-mmlab#1059)

* [Doc]  Add related docs for PoseWarper (open-mmlab#1036)

* add related docs for PoseWarper

* add related readme docs for posewarper

* modify related args in posewarper stage2 config

* modify posewarper stage2 config path

* add description about val_boxes path for data preparation (open-mmlab#1060)

* bump version to v0.21.0 (open-mmlab#1061)

* [Feature] Add ViPNAS_Mbv3 wholebody model (open-mmlab#1055)

* add vipnas mbv3 coco_wholebody

* add vipnas mbv3 coco_wholebody md&yml

* fix lint

Co-authored-by: ly015 <liyining0712@gmail.com>

Co-authored-by: Lumin <30328525+luminxu@users.noreply.github.com>
Co-authored-by: zengwang430521 <zengwang430521@gmail.com>
Co-authored-by: Jas <jinsheng@sensetime.com>
Co-authored-by: jin-s13 <jinsheng13@foxmail.com>
Co-authored-by: Qikai Li <87690686+liqikai9@users.noreply.github.com>
Co-authored-by: QwQ2000 <396707050@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merging PR waited for merging WIP Work in process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants