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

[Feature] support torchserve #160

Merged
merged 11 commits into from
Apr 27, 2022
Merged

[Feature] support torchserve #160

merged 11 commits into from
Apr 27, 2022

Conversation

nijkah
Copy link
Contributor

@nijkah nijkah commented Mar 25, 2022

Motivation

closes #38
Support to deploy to torchserve

Modification

add tools and sample Dockerfile

Use cases (Optional)

Checklist

  1. Pre-commit or other linting tools are used to fix the potential lint issues.
  2. The modification is covered by complete unit tests. If not, please add more unit test to ensure the correctness.
  3. The documentation has been modified accordingly, like docstring or example tutorials.
  • test tools/deployment/mmrotate2serve.py with configs/rotated_faster_rcnn/rotated_faster_rcnn_r50_fpn_1x_dota_le90.py
  • test Dockerfile and torchserve
  • add a documentation

Bump version to v0.1.1
@zytx121
Copy link
Collaborator

zytx121 commented Apr 2, 2022

Hi @nijkah
Good job! Thanks for your PR.

@codecov-commenter
Copy link

codecov-commenter commented Apr 2, 2022

Codecov Report

Merging #160 (141a62c) into dev (3a2dfbf) will increase coverage by 0.42%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##              dev     #160      +/-   ##
==========================================
+ Coverage   28.88%   29.30%   +0.42%     
==========================================
  Files         109      113       +4     
  Lines        7074     7298     +224     
  Branches     1064     1110      +46     
==========================================
+ Hits         2043     2139      +96     
- Misses       4961     5083     +122     
- Partials       70       76       +6     
Flag Coverage Δ
unittests 29.28% <ø> (+0.41%) ⬆️

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

Impacted Files Coverage Δ
mmrotate/apis/train.py 0.00% <0.00%> (ø)
mmrotate/utils/__init__.py 100.00% <0.00%> (ø)
mmrotate/models/dense_heads/__init__.py 100.00% <0.00%> (ø)
mmrotate/core/bbox/assigners/__init__.py 100.00% <0.00%> (ø)
mmrotate/core/bbox/iou_calculators/__init__.py 100.00% <0.00%> (ø)
mmrotate/models/dense_heads/rotated_atss_head.py 12.65% <0.00%> (ø)
mmrotate/utils/setup_env.py 92.00% <0.00%> (ø)
mmrotate/core/bbox/assigners/atss_obb_assigner.py 17.46% <0.00%> (ø)
mmrotate/utils/compat_config.py 78.33% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a2dfbf...141a62c. Read the comment docs.

@nijkah nijkah changed the title [WIP] support torchserve [Feature] support torchserve Apr 3, 2022
@nijkah nijkah marked this pull request as ready for review April 3, 2022 10:38
@yangxue0827 yangxue0827 requested a review from zytx121 April 3, 2022 10:52
@yangxue0827 yangxue0827 added feature and removed WIP labels Apr 3, 2022
@nijkah
Copy link
Contributor Author

nijkah commented Apr 3, 2022

Note that

  1. I added brief document for torchserve in English. But I cannot speak Chinese. Any help would be appreciated.
  2. There is a version issue in torchserve. See this issue. It should be fixed in mmdetection too. There are three options. I followed the second one.
    1. Upgrade the version of torch>=1.8.1
    1. Limit the version of torchserve==0.2.0
    1. Wait for the latest torchserve to fix the issue.
  1. There is a segmentation postprocessing part in tools/deployment/mmrotate_handler.py. Should I delete this?

Copy link
Collaborator

@liuyanyi liuyanyi left a comment

Choose a reason for hiding this comment

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

LGTM

class MMRotateHandler(BaseHandler):
threshold = 0.5

def initialize(self, context):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Docstring is required, like-wise for preprocess, inference and postprocess.

docs/en/useful_tools.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@yangxue0827 yangxue0827 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@zytx121 zytx121 left a comment

Choose a reason for hiding this comment

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

LGTM

@ZwwWayne ZwwWayne merged commit e555fa2 into open-mmlab:dev Apr 27, 2022
@nijkah nijkah deleted the feature/serve branch April 27, 2022 15:35
@zytx121 zytx121 mentioned this pull request Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

deploy to torchserve
6 participants