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(psim)!: preapre settings to launch localization modules on psim #1094

Conversation

yuki-takagi-66
Copy link
Contributor

@yuki-takagi-66 yuki-takagi-66 commented Jul 26, 2024

Description

This PR is a part of the following issue:
#1100

This PR provides launch settings to launch Localization modules on psim.
By setting localization_sim_mode as pose_twist_estimator, most of localization modules without ndt_localizer will be launched.

Note that the frequency of ekf_localizer and vehicle_sim are required to handle carefully. Frequency setting is under analysis and consideration.

universe PR: autowarefoundation/autoware.universe#8212

Tests performed

psim tests and scenario simulator

https://evaluation.tier4.jp/evaluation/reports/16cc961f-f08b-527a-b2fa-120dd899dd5d?project_id=prd_jt (internal link)

Note that the scenario simulator is not supported by this PR.

Effects on system behavior

Not applicable.

Interface changes

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.

@github-actions github-actions bot added component:simulation Virtual environment setups and simulations. (auto-assigned) component:planning Route planning, decision-making, and navigation. (auto-assigned) labels Jul 26, 2024
@yuki-takagi-66 yuki-takagi-66 changed the title Takagi/launch localization on psim feat(psim)!: enabling to launch Localization modules on psim Jul 26, 2024
@yuki-takagi-66 yuki-takagi-66 marked this pull request as ready for review July 26, 2024 07:48
@yuki-takagi-66 yuki-takagi-66 force-pushed the takagi/launch_localization_on_psim branch from b82c944 to e1fc9cb Compare July 26, 2024 07:55
@mitsudome-r
Copy link
Member

Could you explain why you want to launch Localization when using Psim?
My understanding is that PSim is used to test Planning/Control when Perception/Localization outputs are ideal.
Are there any reasons why we are launching localization modules instead of changing algorithms inside PSim for calculating odometry and acceleration?

My concern is that if a developer launch psim locally with scenario_simulator=false, there would be a risk that a bug inside localization modules (e.g., ekf) would impact the result of planning/control modules, which would ruin the purpose of psim.

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.

I will block it to prevent it from being merged.

While PSim used the Component Interface, it is concerning that it will now depend on the internal interfaces of Localization. The role of PSim, as I understand it, is to test Planning/Control under the ideal outputs of Localization/Perception. If its role is to be expanded this much, I think it warrants further discussion.

@yuki-takagi-66 yuki-takagi-66 force-pushed the takagi/launch_localization_on_psim branch from 1ba91ff to 5eb07c6 Compare July 29, 2024 09:23
@yuki-takagi-66 yuki-takagi-66 force-pushed the takagi/launch_localization_on_psim branch from dcfffc8 to 1a91302 Compare August 14, 2024 03:15
@github-actions github-actions bot removed the component:planning Route planning, decision-making, and navigation. (auto-assigned) label Aug 14, 2024
@yuki-takagi-66 yuki-takagi-66 force-pushed the takagi/launch_localization_on_psim branch from 1a91302 to 1984332 Compare August 14, 2024 05:07
@github-actions github-actions bot added component:system System design and integration. (auto-assigned) component:control Vehicle control algorithms and mechanisms. (auto-assigned) component:planning Route planning, decision-making, and navigation. (auto-assigned) labels Aug 14, 2024
@yuki-takagi-66 yuki-takagi-66 changed the title feat(psim)!: enabling to launch Localization modules on psim WIP feat(psim)!: enabling to launch Localization modules on psim Aug 14, 2024
Signed-off-by: Yuki Takagi <[email protected]>
Signed-off-by: Yuki Takagi <[email protected]>
@yuki-takagi-66 yuki-takagi-66 force-pushed the takagi/launch_localization_on_psim branch from 1984332 to ef76bc9 Compare August 14, 2024 09:08
@github-actions github-actions bot removed component:system System design and integration. (auto-assigned) component:control Vehicle control algorithms and mechanisms. (auto-assigned) labels Aug 14, 2024
Signed-off-by: Yuki Takagi <[email protected]>
@yuki-takagi-66 yuki-takagi-66 changed the title WIP feat(psim)!: enabling to launch Localization modules on psim feat(psim)!: enabling to launch localization modules on psim Aug 15, 2024
@yuki-takagi-66 yuki-takagi-66 changed the title feat(psim)!: enabling to launch localization modules on psim feat(psim)!: preapre settings to launch localization modules on psim Aug 15, 2024
@yuki-takagi-66
Copy link
Contributor Author

@yukkysaito
Thank you.
As we discussed internally, we need to achieve the following two goals:
"maintain a simulator environment whose only dependent module is planning/control" and "evaluate planning/control modules, considering the characteristics of localization in a closed loop."
To achieve these two goals, the preceding PR #1106 provided a mode-switching feature supporting three or more states.

@yukkysaito yukkysaito self-requested a review August 19, 2024 01:59
@yuki-takagi-66 yuki-takagi-66 merged commit 713aa97 into autowarefoundation:main Aug 19, 2024
21 of 25 checks passed
@yuki-takagi-66 yuki-takagi-66 deleted the takagi/launch_localization_on_psim branch August 19, 2024 08:51
kyoichi-sugahara pushed a commit to tier4/autoware_launch that referenced this pull request Aug 27, 2024
yuki-takagi-66 added a commit to tier4/autoware_launch that referenced this pull request Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:planning Route planning, decision-making, and navigation. (auto-assigned) component:simulation Virtual environment setups and simulations. (auto-assigned) run:build-and-test-differential
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants