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(planning_validator): add planning validator package #1947

Merged
merged 33 commits into from
Jan 20, 2023

Conversation

TakaHoribe
Copy link
Contributor

@TakaHoribe TakaHoribe commented Sep 24, 2022

Description

For the Issue: #2605

This PR adds a new planning_validator pkg.

motivation

  • Invalid trajectory (e.g. contains infinite values, shape is broken, etc) should be detected in the planning module and handled in an appropriate way.
  • the planning_validator will be put at the last stage in the planning module to check the validity of the planning output.

Design

  • check some metrics
  • when an invalid part is found, the plannig_validator will do some of the following list based on the parameter:
    • publish a Diagnostic msg
    • publish last validated trajectory / publish the invalid trajectory as it is / publish nothing

Related links

autoware_launch: https://github.com/tier4/autoware_launch/pull/675
(for TIER IV internal tracker: https://tier4.atlassian.net/browse/T4PB-18620)

Tests performed

Now some metrics can be seen here.

planning_vaidator_normal_4-2022-09-24_15.07.49.mp4

Invalid case check:
it is confirmed this PR works for the invalid trajectory. This is the case when the trajectory shape is broken when generating lane change trajectory. (For TIER IV INTERNAL, it can be found in this ticket. )

planning_validator_test-2023-01-02_12.03.12.mp4

Notes for reviewers

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.

@codecov
Copy link

codecov bot commented Sep 24, 2022

Codecov Report

Base: 11.37% // Head: 29.44% // Increases project coverage by +18.06% 🎉

Coverage data is based on head (bf847ac) compared to base (0305906).
Patch coverage: 45.84% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1947       +/-   ##
===========================================
+ Coverage   11.37%   29.44%   +18.06%     
===========================================
  Files        1277       12     -1265     
  Lines       89317      917    -88400     
  Branches    23653      277    -23376     
===========================================
- Hits        10160      270     -9890     
+ Misses      68386      442    -67944     
+ Partials    10771      205    -10566     
Flag Coverage Δ
differential 29.44% <45.84%> (?)
total ?
Impacted Files Coverage Δ
...jectory_publisher/invalid_trajectory_publisher.cpp 0.00% <0.00%> (ø)
...idator/test/src/test_planning_validator_pubsub.cpp 35.29% <35.29%> (ø)
...idator/test/src/test_planning_validator_helper.cpp 37.93% <37.93%> (ø)
...ning/planning_validator/src/planning_validator.cpp 45.60% <45.60%> (ø)
...tor/test/src/test_planning_validator_functions.cpp 50.94% <50.94%> (ø)
planning/planning_validator/src/utils.cpp 54.90% <54.90%> (ø)
planning/planning_validator/src/debug_marker.cpp 65.00% <65.00%> (ø)
planning/planning_validator/test/src/test_main.cpp 66.66% <66.66%> (ø)
...idator/include/planning_validator/debug_marker.hpp 100.00% <100.00%> (ø)
.../_planning_debug_tools_s.ep.rosidl_typesupport_c.c
... and 1273 more

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.

@taikitanaka3
Copy link
Contributor

@TakaHoribe
TODO in description is done by planning_error_monitor or maybe by planning evaluator.

@TakaHoribe
Copy link
Contributor Author

@taikitanaka3 right. This package is based on the planning_error_monitor and his all features are (will be) inherited to this package.

@TakaHoribe
Copy link
Contributor Author

@TakaHoribe

1 similar comment
@TakaHoribe
Copy link
Contributor Author

@TakaHoribe

@github-actions github-actions bot added type:documentation Creating or refining documentation. (auto-assigned) component:planning Route planning, decision-making, and navigation. (auto-assigned) labels Dec 31, 2022
@github-actions github-actions bot added the component:system System design and integration. (auto-assigned) label Jan 2, 2023
@github-actions github-actions bot added the component:launch Launch files, scripts and initialization tools. (auto-assigned) label Jan 2, 2023
@TakaHoribe TakaHoribe changed the title [WIP] feat(planning_validator): add planning validator package feat(planning_validator): add planning validator package Jan 2, 2023
@xmfcx
Copy link
Contributor

xmfcx commented Jan 2, 2023

In addition, the validation of a generated path is also be done by iteratively simulating the control stack on the given trajectory and seeing how well it can follow it by calculating the error.

invalid trajectory (e.g. contains infinite values, shape is broken, etc) should be detected in the planning module

Is this package going to check for just these?

Maybe it'd be better if we can list the features that this node can validate.

@taikitanaka3
Copy link
Contributor

@TakaHoribe
In my opinion, this validator package should use trajectory from obstacle cruise and output trajectory for smoother, because smootheris necessary to calculate desired motion (eg. stop, cruise, accelerate) using previous trajectory.

@TakaHoribe
Copy link
Contributor Author

it'd be better if we can list the features that this node can validate.

@xmfcx Thank you for the good advice. The current implementation checks the following items:

Ported from planning_error_monitor

  • invalid field (e.g. Inf, Nan)
  • trajectory points interval (invalid if the distance of trajectory point is too large, like 100m)
  • curvature (invalid if the trajectory has too sharp turns that is not feasible for the given vehicle kinematics)

Implemented in this PR

  • relative angle in trajectory points (invalid if the yaw angle changes too fast in the sequence of trajectory points)
  • lateral acceleration (invalid if the expected lateral acceleration/deceleration is too large)
  • longitudinal acceleration/deceleration (invalid if the acceleration/deceleration is too large)
  • steering angle (expected value estimated from curvature)
  • steering angle rate (expected value estimated from curvature)
  • velocity deviation (invalid if the planning velocity is too far from the ego velocity)
  • distance deviation (invalid if the ego is too far from the trajectory)

I'll list them in the readme as well. Any comments are welcome.

@TakaHoribe
Copy link
Contributor Author

In my opinion, this validator package should use trajectory from obstacle cruise and output trajectory for smoother, because smootheris necessary to calculate desired motion (eg. stop, cruise, accelerate) using previous trajectory.

@taikitanaka3 Sorry I don't get your point. Does your idea solve the original motivation: "we don't want to send the invalid trajectory to the controller"?

@taikitanaka3
Copy link
Contributor

taikitanaka3 commented Jan 3, 2023

@TakaHoribe
partly yes, but my concern is Is it possible to drive using previous trajectory without smoother all the time.
-> afeter discussion with horibesan this pkg is just to validate trajectory and not to guarantee behavior when trajectory is invalid.

@TakaHoribe TakaHoribe marked this pull request as ready for review January 10, 2023 02:10
Signed-off-by: Takamasa Horibe <[email protected]>
Signed-off-by: Takamasa Horibe <[email protected]>
This reverts commit e81c91b.
Signed-off-by: Takamasa Horibe <[email protected]>
Signed-off-by: Takamasa Horibe <[email protected]>
Signed-off-by: Takamasa Horibe <[email protected]>
Signed-off-by: Takamasa Horibe <[email protected]>
Signed-off-by: Takamasa Horibe <[email protected]>
Signed-off-by: Takamasa Horibe <[email protected]>
Signed-off-by: Takamasa Horibe <[email protected]>
Signed-off-by: Takamasa Horibe <[email protected]>
Signed-off-by: Takamasa Horibe <[email protected]>
Signed-off-by: Takamasa Horibe <[email protected]>
Signed-off-by: Takamasa Horibe <[email protected]>
Signed-off-by: Takamasa Horibe <[email protected]>
Signed-off-by: Takamasa Horibe <[email protected]>
Signed-off-by: Takamasa Horibe <[email protected]>
Signed-off-by: Takamasa Horibe <[email protected]>
@TakaHoribe TakaHoribe merged commit 3367c0b into autowarefoundation:main Jan 20, 2023
@TakaHoribe TakaHoribe deleted the feature/planning-validator branch January 20, 2023 09:20
{
rclcpp::NodeOptions node_options;

// for planing validator
Copy link
Contributor

Choose a reason for hiding this comment

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

typo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wow

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you
#2707

maxime-clem pushed a commit to maxime-clem/autoware.universe that referenced this pull request Jan 30, 2023
…ndation#1947)

* feat(planning_validator): add planning validator package

Signed-off-by: Takamasa Horibe <[email protected]>

* remove planning_error_monitor

Signed-off-by: Takamasa Horibe <[email protected]>

* pre-commit

Signed-off-by: Takamasa Horibe <[email protected]>

* change launch for planning_validator

Signed-off-by: Takamasa Horibe <[email protected]>

* Revert "remove planning_error_monitor"

This reverts commit 90aed51.

* restore error_monitor file

Signed-off-by: Takamasa Horibe <[email protected]>

* add readme

Signed-off-by: Takamasa Horibe <[email protected]>

* update for debug marker

Signed-off-by: Takamasa Horibe <[email protected]>

* add debug marker

Signed-off-by: Takamasa Horibe <[email protected]>

* fix invalid index error

Signed-off-by: Takamasa Horibe <[email protected]>

* update readme

Signed-off-by: Takamasa Horibe <[email protected]>

* update

Signed-off-by: Takamasa Horibe <[email protected]>

* add code to calc computation time

Signed-off-by: Takamasa Horibe <[email protected]>

* use reference arg

Signed-off-by: Takamasa Horibe <[email protected]>

* Revert "use reference arg"

This reverts commit e81c91b.

* remove return-vector code

Signed-off-by: Takamasa Horibe <[email protected]>

* Revert "add code to calc computation time"

This reverts commit f36c782.

* update debug plot config

Signed-off-by: Takamasa Horibe <[email protected]>

* update readme

Signed-off-by: Takamasa Horibe <[email protected]>

* fix precommit

Signed-off-by: Takamasa Horibe <[email protected]>

* update readme

Signed-off-by: Takamasa Horibe <[email protected]>

* add invalid trajectory handling option

Signed-off-by: Takamasa Horibe <[email protected]>

* fix typo

Signed-off-by: Takamasa Horibe <[email protected]>

* Update README.md

* update comments

Signed-off-by: Takamasa Horibe <[email protected]>

* pre-commit

Signed-off-by: Takamasa Horibe <[email protected]>

* fix typo

Signed-off-by: Takamasa Horibe <[email protected]>

* update

Signed-off-by: Takamasa Horibe <[email protected]>

* use util for marker create

Signed-off-by: Takamasa Horibe <[email protected]>

* fix tests

Signed-off-by: Takamasa Horibe <[email protected]>

* update doc!

Signed-off-by: Takamasa Horibe <[email protected]>

* fix readme

Signed-off-by: Takamasa Horibe <[email protected]>

* update

Signed-off-by: Takamasa Horibe <[email protected]>

Signed-off-by: Takamasa Horibe <[email protected]>
lexavtanke pushed a commit to lexavtanke/autoware.universe that referenced this pull request Jan 31, 2023
…ndation#1947)

* feat(planning_validator): add planning validator package

Signed-off-by: Takamasa Horibe <[email protected]>

* remove planning_error_monitor

Signed-off-by: Takamasa Horibe <[email protected]>

* pre-commit

Signed-off-by: Takamasa Horibe <[email protected]>

* change launch for planning_validator

Signed-off-by: Takamasa Horibe <[email protected]>

* Revert "remove planning_error_monitor"

This reverts commit 90aed51.

* restore error_monitor file

Signed-off-by: Takamasa Horibe <[email protected]>

* add readme

Signed-off-by: Takamasa Horibe <[email protected]>

* update for debug marker

Signed-off-by: Takamasa Horibe <[email protected]>

* add debug marker

Signed-off-by: Takamasa Horibe <[email protected]>

* fix invalid index error

Signed-off-by: Takamasa Horibe <[email protected]>

* update readme

Signed-off-by: Takamasa Horibe <[email protected]>

* update

Signed-off-by: Takamasa Horibe <[email protected]>

* add code to calc computation time

Signed-off-by: Takamasa Horibe <[email protected]>

* use reference arg

Signed-off-by: Takamasa Horibe <[email protected]>

* Revert "use reference arg"

This reverts commit e81c91b.

* remove return-vector code

Signed-off-by: Takamasa Horibe <[email protected]>

* Revert "add code to calc computation time"

This reverts commit f36c782.

* update debug plot config

Signed-off-by: Takamasa Horibe <[email protected]>

* update readme

Signed-off-by: Takamasa Horibe <[email protected]>

* fix precommit

Signed-off-by: Takamasa Horibe <[email protected]>

* update readme

Signed-off-by: Takamasa Horibe <[email protected]>

* add invalid trajectory handling option

Signed-off-by: Takamasa Horibe <[email protected]>

* fix typo

Signed-off-by: Takamasa Horibe <[email protected]>

* Update README.md

* update comments

Signed-off-by: Takamasa Horibe <[email protected]>

* pre-commit

Signed-off-by: Takamasa Horibe <[email protected]>

* fix typo

Signed-off-by: Takamasa Horibe <[email protected]>

* update

Signed-off-by: Takamasa Horibe <[email protected]>

* use util for marker create

Signed-off-by: Takamasa Horibe <[email protected]>

* fix tests

Signed-off-by: Takamasa Horibe <[email protected]>

* update doc!

Signed-off-by: Takamasa Horibe <[email protected]>

* fix readme

Signed-off-by: Takamasa Horibe <[email protected]>

* update

Signed-off-by: Takamasa Horibe <[email protected]>

Signed-off-by: Takamasa Horibe <[email protected]>
Signed-off-by: Alexey Panferov <[email protected]>
1222-takeshi pushed a commit to 1222-takeshi/autoware.universe that referenced this pull request Mar 9, 2023
…ndation#1947)

* feat(planning_validator): add planning validator package

Signed-off-by: Takamasa Horibe <[email protected]>

* remove planning_error_monitor

Signed-off-by: Takamasa Horibe <[email protected]>

* pre-commit

Signed-off-by: Takamasa Horibe <[email protected]>

* change launch for planning_validator

Signed-off-by: Takamasa Horibe <[email protected]>

* Revert "remove planning_error_monitor"

This reverts commit 90aed51.

* restore error_monitor file

Signed-off-by: Takamasa Horibe <[email protected]>

* add readme

Signed-off-by: Takamasa Horibe <[email protected]>

* update for debug marker

Signed-off-by: Takamasa Horibe <[email protected]>

* add debug marker

Signed-off-by: Takamasa Horibe <[email protected]>

* fix invalid index error

Signed-off-by: Takamasa Horibe <[email protected]>

* update readme

Signed-off-by: Takamasa Horibe <[email protected]>

* update

Signed-off-by: Takamasa Horibe <[email protected]>

* add code to calc computation time

Signed-off-by: Takamasa Horibe <[email protected]>

* use reference arg

Signed-off-by: Takamasa Horibe <[email protected]>

* Revert "use reference arg"

This reverts commit e81c91b.

* remove return-vector code

Signed-off-by: Takamasa Horibe <[email protected]>

* Revert "add code to calc computation time"

This reverts commit f36c782.

* update debug plot config

Signed-off-by: Takamasa Horibe <[email protected]>

* update readme

Signed-off-by: Takamasa Horibe <[email protected]>

* fix precommit

Signed-off-by: Takamasa Horibe <[email protected]>

* update readme

Signed-off-by: Takamasa Horibe <[email protected]>

* add invalid trajectory handling option

Signed-off-by: Takamasa Horibe <[email protected]>

* fix typo

Signed-off-by: Takamasa Horibe <[email protected]>

* Update README.md

* update comments

Signed-off-by: Takamasa Horibe <[email protected]>

* pre-commit

Signed-off-by: Takamasa Horibe <[email protected]>

* fix typo

Signed-off-by: Takamasa Horibe <[email protected]>

* update

Signed-off-by: Takamasa Horibe <[email protected]>

* use util for marker create

Signed-off-by: Takamasa Horibe <[email protected]>

* fix tests

Signed-off-by: Takamasa Horibe <[email protected]>

* update doc!

Signed-off-by: Takamasa Horibe <[email protected]>

* fix readme

Signed-off-by: Takamasa Horibe <[email protected]>

* update

Signed-off-by: Takamasa Horibe <[email protected]>

Signed-off-by: Takamasa Horibe <[email protected]>
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:planning Route planning, decision-making, and navigation. (auto-assigned) component:system System design and integration. (auto-assigned) type:documentation Creating or refining documentation. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.