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

refactor(centerpoint, pointpainting): rearrange parameters for ML models and packages #6591

Merged

Conversation

badai-nguyen
Copy link
Contributor

@badai-nguyen badai-nguyen commented Mar 11, 2024

Description

Releated Link

TIER IV INTERNAL LINK

Tests performed

  • Make sure ML model yaml files are downloaded to autoware_data directory by updating autoware and running ./setup-dev-env.sh from autoware/ directory.
  • Confirmed by logging_simulator that centerpoint or pointpainting correctly reflect the parameters defined in autoware_launch and in data_path
  1. Testing using centerpoint:
    ros2 launch autoware_launch logging_simulator.launch.xml map_path:=$HOME/sample_autoware/sample-map vehicle_model:=sample_vehicle sensor_model:=sample_sensor_kit control:=false planning:=false perception:=true localization:=false perception_mode:=camera_lidar_fusion centerpoint_model_name:=centerpoint

image

  1. Testing using centerpoint_tiny
    ros2 launch autoware_launch logging_simulator.launch.xml map_path:=$HOME/sample_autoware/sample-map vehicle_model:=sample_vehicle sensor_model:=sample_sensor_kit control:=false planning:=false perception:=true localization:=false perception_mode:=camera_lidar_fusion centerpoint_model_name:=centerpoint_tiny
    image

  2. Tesing using pointpainting
    ros2 launch autoware_launch logging_simulator.launch.xml map_path:=$HOME/sample_autoware/sample-map vehicle_model:=sample_vehicle sensor_model:=sample_sensor_kit control:=false planning:=false perception:=true localization:=false perception_mode:=camera_lidar_fusion lidar_detection_model:=pointpainting
    image

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.

Signed-off-by: badai-nguyen <[email protected]>
Signed-off-by: badai-nguyen <[email protected]>
Signed-off-by: badai-nguyen <[email protected]>
Signed-off-by: badai-nguyen <[email protected]>
@github-actions github-actions bot added component:perception Advanced sensor data processing and environment understanding. (auto-assigned) component:launch Launch files, scripts and initialization tools. (auto-assigned) labels Mar 11, 2024
Copy link
Contributor

@kminoda kminoda left a comment

Choose a reason for hiding this comment

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

Thank you for the PR!!

Signed-off-by: badai-nguyen <[email protected]>
@badai-nguyen badai-nguyen changed the title refactor:(centerpoint, pointpainting): ML model params rework refactor(centerpoint, pointpainting): rearrange parameters for ML models and packages Mar 13, 2024
@badai-nguyen badai-nguyen marked this pull request as ready for review March 14, 2024 00:15
@badai-nguyen badai-nguyen marked this pull request as draft March 14, 2024 05:30
Signed-off-by: badai-nguyen <[email protected]>
Signed-off-by: badai-nguyen <[email protected]>
@github-actions github-actions bot added the type:documentation Creating or refining documentation. (auto-assigned) label Mar 17, 2024
@badai-nguyen badai-nguyen marked this pull request as ready for review March 18, 2024 00:09
@kminoda kminoda added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Mar 21, 2024
Copy link

codecov bot commented Mar 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 14.78%. Comparing base (35642a6) to head (8973229).
Report is 79 commits behind head on main.

❗ Current head 8973229 differs from pull request most recent head 70f4388. Consider uploading reports for the commit 70f4388 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6591      +/-   ##
==========================================
- Coverage   14.93%   14.78%   -0.16%     
==========================================
  Files        1944     1925      -19     
  Lines      134061   132724    -1337     
  Branches    39891    39521     -370     
==========================================
- Hits        20027    19623     -404     
+ Misses      91735    91127     -608     
+ Partials    22299    21974     -325     
Flag Coverage Δ *Carryforward flag
differential 0.00% <ø> (?)
total 14.78% <ø> (-0.16%) ⬇️ Carriedforward from a8364b4

*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.

@Shin-kyoto
Copy link
Contributor

@badai-nguyen
Can you resolve conflicts?

@badai-nguyen
Copy link
Contributor Author

badai-nguyen commented Apr 18, 2024

@Shin-kyoto @knzo25
cc @miursh @kminoda @tzhong518

I have updated and rearrange some parameters. I also updated the logging_simulator testing results in Tests performed.
Could you review this PR again.

@badai-nguyen badai-nguyen force-pushed the refactor/ml_model_param_rework branch from f12cfd0 to ad45a92 Compare April 24, 2024 07:49
Copy link
Contributor

@miursh miursh 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
Contributor

@knzo25 knzo25 left a comment

Choose a reason for hiding this comment

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

LGTM

@badai-nguyen badai-nguyen force-pushed the refactor/ml_model_param_rework branch from 3544601 to 70f4388 Compare April 26, 2024 11:54
@badai-nguyen
Copy link
Contributor Author

@Shin-kyoto
I fixed your change request so could you review it again?

BTW, I have merged autowarefoundation/autoware#4533 tested this change on Evaluator.

@badai-nguyen badai-nguyen enabled auto-merge (squash) April 26, 2024 12:06
Copy link
Contributor

@Shin-kyoto Shin-kyoto left a comment

Choose a reason for hiding this comment

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

LGTM

@badai-nguyen badai-nguyen merged commit 46905ee into autowarefoundation:main Apr 26, 2024
21 of 22 checks passed
@@ -0,0 +1,98 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

@badai-nguyen This file has a wrong extension. (.schemal.json)

Might be intentional too, I don't know.

Maybe related to autowarefoundation/autoware-github-actions#293

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xmfcx Thank you for your comment. It was my mistake. I will fix it.

karishma1911 pushed a commit to Interplai/autoware.universe that referenced this pull request Jun 3, 2024
…els and packages (autowarefoundation#6591)

* refactor: lidar_centerpoint

Signed-off-by: badai-nguyen <[email protected]>

* refactor: pointpainting

Signed-off-by: badai-nguyen <[email protected]>

* chore: fix launch

Signed-off-by: badai-nguyen <[email protected]>

* chore: fix launch

Signed-off-by: badai-nguyen <[email protected]>

* chore: rearrange params

Signed-off-by: badai-nguyen <[email protected]>

* fix: json-schema-check error

Signed-off-by: badai-nguyen <[email protected]>

* fix: default param

Signed-off-by: badai-nguyen <[email protected]>

* refactor: rename param file

Signed-off-by: badai-nguyen <[email protected]>

* chore: typo

Signed-off-by: badai-nguyen <[email protected]>

* fix: align centerpoint param namespace with pointpainting

Signed-off-by: badai-nguyen <[email protected]>

* fix(centerpoint): add schema json

Signed-off-by: badai-nguyen <[email protected]>

* fix(pointpainting): fix schema json typo

Signed-off-by: badai-nguyen <[email protected]>

* style(pre-commit): autofix

* docs: update pointpainting fusion doc

Signed-off-by: badai-nguyen <[email protected]>

* docs: update lidar centerpoint doc

Signed-off-by: badai-nguyen <[email protected]>

* fix: change omp param

Signed-off-by: badai-nguyen <[email protected]>

* fix:change twist and variance to model params

Signed-off-by: badai-nguyen <[email protected]>

* fix: keep build_only in launch

Signed-off-by: badai-nguyen <[email protected]>

* fix: schema check

Signed-off-by: badai-nguyen <[email protected]>

* chore: temporary remove schema required

Signed-off-by: badai-nguyen <[email protected]>

---------

Signed-off-by: badai-nguyen <[email protected]>
Co-authored-by: Kenzo Lobos Tsunekawa <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
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) run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) type:documentation Creating or refining documentation. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants