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

fix: center point class #4701

Conversation

taikitanaka3
Copy link
Contributor

@taikitanaka3 taikitanaka3 commented Aug 22, 2023

Description

  • add build only option which is necessary to run AWS + Docker
  • fix center point class and add build only option
  • revert to use center point tiny as default

Note

  • tiny is AWF default model
  • the reason we use center point is center point tiny has bug and it was risky to stay still but After fix which @knzo25 made and @shmpwk merged at 461f330
    Therefore there is no reason not to use tiny model.
ros2 launch lidar_centerpoint lidar_centerpoint.launch.xml model_name:=centerpoint build_only:=true
ros2 launch lidar_centerpoint lidar_centerpoint.launch.xml model_name:=centerpoint_tiny build_only:=true

Tests performed

by AWSIM using center point
Screenshot from 2023-08-22 15-53-50

Effects on system behavior

Not applicable.

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.

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.

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

yukke42 and others added 2 commits August 22, 2023 12:15
…utowarefoundation#2677)

* feat(lidar_centerpoint): add build only option for tensorrt engine

Signed-off-by: yukke42 <yukke42@users.noreply.github.com>

* fix typo

Co-authored-by: Daisuke Nishimatsu <42202095+wep21@users.noreply.github.com>

* style(pre-commit): autofix

* chore: add description

Signed-off-by: yukke42 <yukke42@users.noreply.github.com>

* chore: shutdown node

Signed-off-by: yukke42 <yukke42@users.noreply.github.com>

* feat: use tensorrt_commom

Signed-off-by: yukke42 <yukke42@users.noreply.github.com>

* fix: resolve the crash when shutting down the node

Signed-off-by: yukke42 <yusuke.muramatsu@tier4.jp>

* chore: fix typo

Co-authored-by: Kenzo Lobos Tsunekawa <kenzo.lobos@tier4.jp>

* style(pre-commit): autofix

---------

Signed-off-by: yukke42 <yukke42@users.noreply.github.com>
Signed-off-by: yukke42 <yusuke.muramatsu@tier4.jp>
Co-authored-by: Daisuke Nishimatsu <42202095+wep21@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Kenzo Lobos Tsunekawa <kenzo.lobos@tier4.jp>
* fix(lidar_centerpoint): fix class

Signed-off-by: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com>

* feat(lidar_centerpoint): fix range in config for centerpoint

Signed-off-by: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com>

* feat(lidar_centerpoint): fix the ranges and voxel size

Signed-off-by: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com>

---------

Signed-off-by: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com>
@taikitanaka3 taikitanaka3 requested review from knzo25, yukke42 and a team as code owners August 22, 2023 06:33
@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 Aug 22, 2023
@github-actions github-actions bot added the component:launch Launch files, scripts and initialization tools. (auto-assigned) label Aug 22, 2023
@yukkysaito
Copy link
Contributor

@taikitanaka3 @yukke42 Should we set it to tiny as default?

@taikitanaka3
Copy link
Contributor Author

@yukkysaito

  • tiny is AWF default model
  • the reason we use center point is center point tiny has bug and it was risky to stay still but After fix which @knzo25 made and @shmpwk merged 461f330
    there is no reason not to use tiny model.

so I think it's better to change default to tiny.

@codecov
Copy link

codecov bot commented Aug 22, 2023

Codecov Report

❗ No coverage uploaded for pull request base (awsim-stable@a4936b6). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@               Coverage Diff               @@
##             awsim-stable    #4701   +/-   ##
===============================================
  Coverage                ?   11.87%           
===============================================
  Files                   ?     1333           
  Lines                   ?    92763           
  Branches                ?    24649           
===============================================
  Hits                    ?    11011           
  Misses                  ?    70341           
  Partials                ?    11411           
Flag Coverage Δ *Carryforward flag
differential 0.00% <0.00%> (?)
total 11.87% <0.00%> (?) Carriedforward from 0d4e444

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

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

Copy link
Contributor

@shmpwk shmpwk left a comment

Choose a reason for hiding this comment

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

I confirmed that it works properly with AWSIM

@taikitanaka3 taikitanaka3 merged commit 05d091d into autowarefoundation:awsim-stable Aug 23, 2023
@taikitanaka3 taikitanaka3 deleted the fix/center_point_class branch August 23, 2023 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:launch Launch files, scripts and initialization tools. (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