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

[ENHANCE] Bring mmdeploy to action recognition model export & Test optimization of action tasks #1848

Merged
merged 7 commits into from
Mar 9, 2023

Conversation

jaegukhyun
Copy link
Contributor

@jaegukhyun jaegukhyun commented Mar 2, 2023

This PR includes

  1. Bringing mmdeploy to action recognition model export
  2. Test optimization of action tasks

Why this should be changed
Unlike other algorithms in otx, action tasks use manual export procedure, not mmdeploy. For unity and convenience this PR brings mmdeploy as a framework to export action algo models.
And also, action tasks cli test train models for 2 epochs, so it requires more time than other algorithms. Therefore this PR reduces it to 1

Changes

  1. [otx/algorithms/action/adapters/mmaction/models/detectors/fast_rcnn.py]:
    • Change static pooling method: Unfortunately there is an issue regarding to process 3D pooling in openvino with pytorch 1.13(pytorch 1.12 is fine). Therefore this PR change pooling methods to manual method(avgpool -> torch.mean, maxpool -> torch.max)
    • Change building method 2d person detector: mmdeploy requires to build model using its processor so that the monkey patched function works properly. Therefore method to add 2d person detector is changed
    • Add rewriter flags to forward_infer function and unify forward path to simple_test: From using function rewriter in mmdeploy, the forward_infer function can act like forward function during deployment. And this PR unify the forward route so that AVAFastRCNN use same forward path, which is monkey patched in mmdeploy, with 2d person detector.
  2. [otx/algorithms/action/adapters/mmaction/utils/export_utils.py]
    • Create new class Exporter: Previous codes for exporting model is designed as procedural programming. This PR changes as OOP from bringing new class Exporter.
  3. [otx/algorithms/action/tasks/inference.py]
    • As the exporting methods changed, the method to call exporting method should be changed
    • Add _initialized_post_hook to bring deploy_cfg of task
  4. Others
    • deployment.py are added to action classification and action detection model's template directories.
    • Action integration test run training for 1 epoch and remove skip flag for action detection export test

Known Issues

  • In pytorch 1.13 3d pooling do not work in openvino ==> Soloved by temporal manual method

  • In pytorch 1.13 Two stage openvino models return empty inference result ==> Will be solved in CVS104657

  • Action task do not support dump_features(feature vector, saliency map) ==> Will be treated in another PR

  • Bring mmdeploy to mmaction's model

  • Test optimization for action tasks

  • Change unit tests for this PR

@github-actions github-actions bot added ALGO Any changes in OTX Algo Tasks implementation TEST Any changes in tests labels Mar 2, 2023
@jaegukhyun jaegukhyun marked this pull request as ready for review March 3, 2023 02:29
@jaegukhyun jaegukhyun requested a review from a team as a code owner March 3, 2023 02:29
@jaegukhyun jaegukhyun requested review from JihwanEom and cih9088 March 3, 2023 02:31
@jaegukhyun
Copy link
Contributor Author

@cih9088 Could you review overall structure for linking between action tasks and mmdeploy framework, and deployment.py for each action models? Since I'm not familiar with mmdeploy, your review would be helpful. Minor comment will be fixed in this PR and large suggestion such as changing overall architecture will be reflected in another PR after merging this PR

@yunchu
Copy link
Contributor

yunchu commented Mar 3, 2023

@jaegukhyun please set the milestone for this PR. I guess it would be included to the next release (v1.1.0), right?

@jaegukhyun jaegukhyun added this to the 1.1.0 milestone Mar 3, 2023
@jaegukhyun
Copy link
Contributor Author

@jaegukhyun please set the milestone for this PR. I guess it would be included to the next release (v1.1.0), right?

Thank's for the remark, I set it as 1.1.0

@codecov-commenter
Copy link

codecov-commenter commented Mar 3, 2023

Codecov Report

Patch coverage: 93.24% and project coverage change: -0.03 ⚠️

Comparison is base (0f6616e) 80.54% compared to head (d78b2c4) 80.52%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1848      +/-   ##
===========================================
- Coverage    80.54%   80.52%   -0.03%     
===========================================
  Files          481      483       +2     
  Lines        32998    32943      -55     
===========================================
- Hits         26579    26527      -52     
+ Misses        6419     6416       -3     
Impacted Files Coverage Δ
...n/configs/detection/base/base_detection_dynamic.py 0.00% <0.00%> (ø)
...on/configs/detection/base/base_detection_static.py 0.00% <0.00%> (ø)
...on/adapters/mmaction/models/detectors/fast_rcnn.py 100.00% <100.00%> (ø)
...hms/action/adapters/mmaction/utils/export_utils.py 100.00% <100.00%> (+6.71%) ⬆️
otx/algorithms/action/tasks/inference.py 89.16% <100.00%> (+0.69%) ⬆️
...hms/detection/adapters/mmdet/utils/config_utils.py 93.29% <0.00%> (-1.22%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@cih9088 cih9088 left a comment

Choose a reason for hiding this comment

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

I have noticed that task structure is pretty different from cls/det/seg. Would cls/det/seg be refactored in a way where action implemented task?
Additionally, otxdeploy requirement should be added into requirements/action.txt

@github-actions github-actions bot added the DEPENDENCY Any changes in any dependencies (new dep or its version) should be produced via Change Request on PM label Mar 6, 2023
@jaegukhyun
Copy link
Contributor Author

@cih9088 Thank you for your detailed review, I reflected almost of your review. Currently action task do not use mpa side code, therefore I create own export codes. After refactoring mpa codes, I'll make another PR for unifying exporting process

Copy link
Contributor

@JihwanEom JihwanEom left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for hard work!

@goodsong81 goodsong81 changed the title [Enhance] Bring mmdeploy to action recognition model export & Test optimization of action tasks [ENHANCE] Bring mmdeploy to action recognition model export & Test optimization of action tasks Mar 9, 2023
@goodsong81 goodsong81 enabled auto-merge (squash) March 9, 2023 01:38
@goodsong81 goodsong81 merged commit 9e643ac into develop Mar 9, 2023
@goodsong81 goodsong81 deleted the jg/action-mmdeploy branch March 9, 2023 02:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ALGO Any changes in OTX Algo Tasks implementation DEPENDENCY Any changes in any dependencies (new dep or its version) should be produced via Change Request on PM TEST Any changes in tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants