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

feat: add tensorrt_yolox package #2370

Merged
merged 12 commits into from
Nov 30, 2022

Conversation

manato
Copy link
Contributor

@manato manato commented Nov 24, 2022

Description

This PR aims to add the tensorrt_yolox package into the series of perception modules in autoware.universe.
The tensorrt_yolox package that works with autoware has been published in separated repository. This PR integrates the package's contents into autoware.universe.

Related links

Tests performed

# build
colcon build --symlink-install --cmake-args -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_BUILD_TYPE=Release --packages-up-to tensorrt_yolox

# run
ros2 run tensorrt_yolox tensorrt_yolox_node_exe --ros-args -p model_path:=install/tensorrt_yolox/share/tensorrt_yolox/data/yolox-tiny.onnx -p label_path:=install/tensorrt_yolox/share/tensorrt_yolox/data/label.txt -r /tensorrt_yolox/in/image:=/image_raw

Notes for reviewers

I've added small modifications listed below from the original repository

  • treating labels as case insensitive
  • treating the PERSON label in the same way as the PEDESTRIAN label for COCO dataset compatibility

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

  • The PR follows the pull request guidelines.
  • The PR has been properly tested.
  • The PR has been reviewed by the code owners.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

  • There are no open discussions or they are tracked via tickets.
  • The PR is ready for merge.

After all checkboxes are checked, anyone who has write access can merge the PR.

@github-actions github-actions bot added type:documentation Creating or refining documentation. (auto-assigned) component:perception Advanced sensor data processing and environment understanding. (auto-assigned) labels Nov 24, 2022
@manato manato marked this pull request as ready for review November 28, 2022 02:07
@wep21
Copy link
Contributor

wep21 commented Nov 29, 2022

I think it's better to separate tensorrt_yolox, tensorrt_common, cuda_utils because another package can use common functions in tensorrt_common, cuda_utils.

@manato
Copy link
Contributor Author

manato commented Nov 30, 2022

I think it's better to separate tensorrt_yolox, tensorrt_common, cuda_utils because another package can use common functions in tensorrt_common, cuda_utils.

@wep21
I really appreciate your suggestion. I agree with you, so let me confirm where to put tensorrt_common and cuda_utils for further modification. Do you think autoware.universe/common/ is the proper place to put them? If you think a different directory is preferable, I would appreciate it if you let me know.

@github-actions github-actions bot added the component:common Common packages from the autoware-common repository. (auto-assigned) label Nov 30, 2022
@yukkysaito yukkysaito self-requested a review November 30, 2022 10:19
Copy link
Contributor

@yukkysaito yukkysaito left a comment

Choose a reason for hiding this comment

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

LGTM

@wep21
Copy link
Contributor

wep21 commented Nov 30, 2022

change format to autoware style

@miursh
Copy link
Contributor

miursh commented Nov 30, 2022

@manato You should add your name to maintainer in package.xml.
Also, could you fix CI error?

manato and others added 3 commits November 30, 2022 20:34
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
@codecov
Copy link

codecov bot commented Nov 30, 2022

Codecov Report

Base: 10.45% // Head: 10.40% // Decreases project coverage by -0.05% ⚠️

Coverage data is based on head (440181a) compared to base (eb9fcec).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2370      +/-   ##
==========================================
- Coverage   10.45%   10.40%   -0.06%     
==========================================
  Files        1253     1259       +6     
  Lines       91273    91714     +441     
  Branches    20935    20935              
==========================================
  Hits         9541     9541              
- Misses      71593    72034     +441     
  Partials    10139    10139              
Flag Coverage Δ *Carryforward flag
differential 0.00% <0.00%> (?)
total 10.42% <ø> (ø) Carriedforward from eb9fcec

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
...common/include/tensorrt_common/tensorrt_common.hpp 0.00% <0.00%> (ø)
common/tensorrt_common/src/tensorrt_common.cpp 0.00% <0.00%> (ø)
...rt_yolox/include/tensorrt_yolox/tensorrt_yolox.hpp 0.00% <0.00%> (ø)
perception/tensorrt_yolox/src/tensorrt_yolox.cpp 0.00% <0.00%> (ø)
...ception/tensorrt_yolox/src/tensorrt_yolox_node.cpp 0.00% <0.00%> (ø)
...rt_yolox/src/yolox_single_image_inference_node.cpp 0.00% <0.00%> (ø)

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.

@manato
Copy link
Contributor Author

manato commented Nov 30, 2022

@manato You should add your name to maintainer in package.xml. Also, could you fix CI error?

@miursh
Thank you very much for pointing this out. I completely missed it. I added maintainer descriptions in 4fa36ae

Also, the CI errors seem to be gone with the kind help of @yukke42 and @wep21 . I really appreciate their grateful support.

@wep21 wep21 merged commit 03de605 into autowarefoundation:main Nov 30, 2022
@manato manato deleted the feat/tensorrt_yolox branch November 30, 2022 12:25
tzhong518 pushed a commit to tier4/autoware.universe that referenced this pull request Dec 9, 2022
* feat: clone tensorrt_yolox into autoware.universe

* build: clone cuda_utils and tensorrt_common into lib

* feat: support downloading pretrained model during build

* fix: make label being case insensitive

Now, all labels are treated as upper case

* fix: support COCO-style label

* docs: add README for tensorrt_yolox package

* fix: remove pre-commit error

* build: replace some boilerplate code by autoware_cmake

* refactor: divide tensorrt_common and cuda_utils into separated packages

* fix: add maintainer

* fix: correct typo

* use autoware lint common

Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>

Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
Co-authored-by: Manato HIRABAYASHI <manato.hirabayashi@tier4.jp>
Co-authored-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
HansRobo pushed a commit to HansRobo/autoware.universe that referenced this pull request Dec 16, 2022
* feat: clone tensorrt_yolox into autoware.universe

* build: clone cuda_utils and tensorrt_common into lib

* feat: support downloading pretrained model during build

* fix: make label being case insensitive

Now, all labels are treated as upper case

* fix: support COCO-style label

* docs: add README for tensorrt_yolox package

* fix: remove pre-commit error

* build: replace some boilerplate code by autoware_cmake

* refactor: divide tensorrt_common and cuda_utils into separated packages

* fix: add maintainer

* fix: correct typo

* use autoware lint common

Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>

Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
Co-authored-by: Manato HIRABAYASHI <manato.hirabayashi@tier4.jp>
Co-authored-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
Signed-off-by: Kotaro Yoshimoto <pythagora.yoshimoto@gmail.com>
kminoda pushed a commit to kminoda/autoware.universe that referenced this pull request Jan 6, 2023
* feat: clone tensorrt_yolox into autoware.universe

* build: clone cuda_utils and tensorrt_common into lib

* feat: support downloading pretrained model during build

* fix: make label being case insensitive

Now, all labels are treated as upper case

* fix: support COCO-style label

* docs: add README for tensorrt_yolox package

* fix: remove pre-commit error

* build: replace some boilerplate code by autoware_cmake

* refactor: divide tensorrt_common and cuda_utils into separated packages

* fix: add maintainer

* fix: correct typo

* use autoware lint common

Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>

Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
Co-authored-by: Manato HIRABAYASHI <manato.hirabayashi@tier4.jp>
Co-authored-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
Signed-off-by: kminoda <koji.minoda@tier4.jp>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:common Common packages from the autoware-common repository. (auto-assigned) component:perception Advanced sensor data processing and environment understanding. (auto-assigned) type:documentation Creating or refining documentation. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants