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(autoware_planning_evaluator): add resampled_relative_angle metrics #10020

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

Kazunori-Nakajima
Copy link
Contributor

Description

Added metrics to look at angular differences in trajectory points.
This metric enables detection of severe route misalignments, such as bends of more than 90° in local trajectory.

Related links

Parent Issue:

  • Link

How was this PR tested?

simplescreenrecorder-2025-01-27_09.08.46.mp4

Notes for reviewers

None.

Interface changes

None.

Effects on system behavior

None.

@github-actions github-actions bot added the component:evaluator Evaluation tools for planning, localization etc. (auto-assigned) label Jan 27, 2025
Copy link

github-actions bot commented Jan 27, 2025

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

Comment on lines 92 to 96
// Resample trajctory points
std::vector<TrajectoryPoint> resample_traj_points{};
for (size_t idx = 0; idx < traj.points.size(); idx = idx + 3) {
resample_traj_points.push_back(traj.points.at(idx));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this necessary for calculation costs?

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 very much for comment!
It seems to not necessary. This processing time is at most 0.5 ms.
I deleted this code -> 1b29489

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks!
0.5ms seems not so problem

( for reduce proccesing time more: https://star4.slack.com/archives/C03QW0GU6P7/p1737942266253849?thread_ts=1737937464.395459&cid=C03QW0GU6P7 )

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 very much!
I fixed it.

default.mp4

Comment on lines 87 to 90
// We need at least three points to compute relative angle
if (traj.points.size() < 2) {
return stat;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

now we can delete this lines

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I deleted!

@kosuke55
Copy link
Contributor

  • rename metrics name to resampled_relative_angle
  • fix ci check

@kosuke55 kosuke55 added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Jan 27, 2025
@kosuke55 kosuke55 changed the title feat(autoware_planning_evaluator): add new large_relative_angle metrics feat(autoware_planning_evaluator): add resampled_relative_angle metrics Jan 27, 2025
@kosuke55 kosuke55 force-pushed the feat/planning_evaluator/add_large_relative_angle branch from 98463cc to 9246146 Compare January 27, 2025 14:14
@kosuke55
Copy link
Contributor

@Kazunori-Nakajima
ci unit test fails, it seems that you need to update test code for using vehicle_info in it.

image

@Kazunori-Nakajima
Copy link
Contributor Author

@kosuke55

ci unit test fails, it seems that you need to update test code for using vehicle_info in it.

I fix unit test, and add resampled_relative_angle unit test -> 4c90661
Could you please review unit test? Test is all pass in local.

colcon test --packages-select autoware_planning_evaluator --event-handlers console_cohesion+

image

Copy link

codecov bot commented Jan 28, 2025

Codecov Report

Attention: Patch coverage is 83.33333% with 3 lines in your changes missing coverage. Please review.

Project coverage is 29.27%. Comparing base (461dbc1) to head (7a4a59e).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
...e_planning_evaluator/src/motion_evaluator_node.cpp 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10020      +/-   ##
==========================================
+ Coverage   29.21%   29.27%   +0.06%     
==========================================
  Files        1439     1440       +1     
  Lines      108115   108009     -106     
  Branches    42638    42545      -93     
==========================================
+ Hits        31588    31624      +36     
+ Misses      73485    73342     -143     
- Partials     3042     3043       +1     
Flag Coverage Δ *Carryforward flag
differential 9.24% <83.33%> (?)
total 29.27% <ø> (+0.05%) ⬆️ Carriedforward from e062639

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

Comment on lines 104 to 110
// Calc diff yaw between base pose yaw and target pose yaw
const double front_diff_yaw =
autoware::universe_utils::normalizeRadian(front_target_yaw - base_yaw);
const double back_diff_yaw =
autoware::universe_utils::normalizeRadian(back_target_yaw - base_yaw);

stat.add(std::abs(std::max(front_diff_yaw, back_diff_yaw)));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Calc diff yaw between base pose yaw and target pose yaw
const double front_diff_yaw =
autoware::universe_utils::normalizeRadian(front_target_yaw - base_yaw);
const double back_diff_yaw =
autoware::universe_utils::normalizeRadian(back_target_yaw - base_yaw);
stat.add(std::abs(std::max(front_diff_yaw, back_diff_yaw)));
// Calc diff yaw between base pose yaw and target pose yaw
const double front_diff_yaw =
std::abs(autoware::universe_utils::normalizeRadian(front_target_yaw - base_yaw));
const double back_diff_yaw =
std::abs(autoware::universe_utils::normalizeRadian(back_target_yaw - base_yaw));
stat.add(std::max(front_diff_yaw, back_diff_yaw));

Copy link
Contributor

Choose a reason for hiding this comment

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

should compare abs value

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 very much!
That true, I fixed.7a4a59e

Signed-off-by: Kasunori-Nakajima <[email protected]>
@kosuke55
Copy link
Contributor

@kyoichi-sugahara @maxime-clem
could you approve as a code owner?

* @return calculated statistics
*/
Accumulator<double> calcTrajectoryResampledRelativeAngle(
const Trajectory & traj, const double & vehicle_length_m);
Copy link
Contributor

Choose a reason for hiding this comment

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

(minor) primitive types do not need to be passed by reference.

Suggested change
const Trajectory & traj, const double & vehicle_length_m);
const Trajectory & traj, const double vehicle_length_m);

const auto resample_offset = vehicle_length_m / 2;
const auto arc_length =
autoware::motion_utils::calcSignedArcLengthPartialSum(traj.points, 0, traj.points.size());
for (size_t base_id = 0; base_id < arc_length.size() - 1; ++base_id) {
Copy link
Contributor

Choose a reason for hiding this comment

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

(minor) since size_t is an unsigned type, if arc_length.size() is equal to 0 then a bug will occur when doing arc_length.size() - 1. Therefore I recommend adding one on the left side of the comparison instead.

Suggested change
for (size_t base_id = 0; base_id < arc_length.size() - 1; ++base_id) {
for (size_t base_id = 0; base_id + 1 < arc_length.size(); ++base_id) {

Comment on lines +98 to +102
for (size_t target_id = base_id; target_id < arc_length.size(); ++target_id) {
if (arc_length[target_id] >= arc_length[base_id] + resample_offset) {
// Get target pose yaw
const double front_target_yaw = tf2::getYaw(traj.points.at(target_id).pose.orientation);
const double back_target_yaw = tf2::getYaw(traj.points.at(target_id - 1).pose.orientation);
Copy link
Contributor

Choose a reason for hiding this comment

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

When base_id = 0, then the first value of target_id will be 0. I suspect there could be a bug here when doing target_id - 1.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, thanks for your review

Comment on lines +98 to +110
for (size_t target_id = base_id; target_id < arc_length.size(); ++target_id) {
if (arc_length[target_id] >= arc_length[base_id] + resample_offset) {
// Get target pose yaw
const double front_target_yaw = tf2::getYaw(traj.points.at(target_id).pose.orientation);
const double back_target_yaw = tf2::getYaw(traj.points.at(target_id - 1).pose.orientation);

// Calc diff yaw between base pose yaw and target pose yaw
const double front_diff_yaw =
std::abs(autoware::universe_utils::normalizeRadian(front_target_yaw - base_yaw));
const double back_diff_yaw =
std::abs(autoware::universe_utils::normalizeRadian(back_target_yaw - base_yaw));

stat.add(std::max(front_diff_yaw, back_diff_yaw));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for (size_t target_id = base_id; target_id < arc_length.size(); ++target_id) {
if (arc_length[target_id] >= arc_length[base_id] + resample_offset) {
// Get target pose yaw
const double front_target_yaw = tf2::getYaw(traj.points.at(target_id).pose.orientation);
const double back_target_yaw = tf2::getYaw(traj.points.at(target_id - 1).pose.orientation);
// Calc diff yaw between base pose yaw and target pose yaw
const double front_diff_yaw =
std::abs(autoware::universe_utils::normalizeRadian(front_target_yaw - base_yaw));
const double back_diff_yaw =
std::abs(autoware::universe_utils::normalizeRadian(back_target_yaw - base_yaw));
stat.add(std::max(front_diff_yaw, back_diff_yaw));
// Get arc length offset from base point
const double needed_arc = arc_length[base_id] + resample_offset;
for (size_t target_id = base_id; target_id < arc_length.size(); ++target_id) {
// Linear interpolate between points when needed_arc is found
if (arc_length[target_id] >= needed_arc) {
// Get arc lengths for linear interpolation
const double arc_1 = arc_length[target_id - 1];
const double arc_2 = arc_length[target_id];
// Get linear interpolation ratio from 0 to 1
double ratio = (needed_arc - arc_1) / (arc_2 - arc_1);
ratio = std::max(0.0, std::min(1.0, ratio));
const double yaw_1 = tf2::getYaw(traj.points[target_id - 1].pose.orientation);
const double yaw_2 = tf2::getYaw(traj.points[target_id].pose.orientation);
// Linear interpolate yaw
double diff = autoware::universe_utils::normalizeRadian(yaw_2 - yaw_1);
double interp_yaw = yaw_1 + diff * ratio;
interp_yaw = autoware::universe_utils::normalizeRadian(interp_yaw);
const double diff_yaw =
std::abs(autoware::universe_utils::normalizeRadian(interp_yaw - base_yaw));
stat.add(diff_yaw);

Minor comment: Please ignore this depending on the anomalies you want to detect.
if there is motivation to keep distance intervals constant, I suggest that simple linear interpolation might be option. Here's an example as a suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:evaluator Evaluation tools for planning, localization etc. (auto-assigned) run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci)
Projects
Status: To Triage
Development

Successfully merging this pull request may close these issues.

4 participants