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(obstacle_cruise_planner): add separate parameters for cruise and stop margins #1089

Conversation

ahmeddesokyebrahim
Copy link
Contributor

@ahmeddesokyebrahim ahmeddesokyebrahim commented Jul 23, 2024

Description

Closes autowarefoundation/autoware.universe#7922

Related links

autowarefoundation/autoware.universe#7485

Tests performed

Using scenario simulation

❗ Note ❗
In this previous scenario, the safe distance is updated to be 12.5 m following the 3-seconds inter-vehicle time-distance rule in performance requirements here

Before this PR:

2024-07-23.15-28-31.mp4

After this PR:

2024-07-23.15-02-16.mp4

Notes for reviewers

This PR is used by an autoware.universe PR

Interface changes

ROS Topic Changes

N.A.

ROS Parameter Changes

Parameter Name Default Value Update Description
common.cruise_safe_distance_method 0.5 cruise safe distance margin used by RSS equation as a small constant to avoid zero distance
common.stop_safe_distance_method 6.0 stop safe distance margin used as previous safe_distance_method for stop purposes
common.terminal_stop_safe_distance_margin 3.0 Stop margin at the goal, used as previous terminal_safe_distance_margin

Effects on system behavior

Improving obstacle_cruise_planner behavior in cruising scenarios, mitigating any mitigate any unnecessary long safe distance ego and npc and satisfying performance requirements

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.

@@ -12,8 +12,9 @@
idling_time: 2.0 # idling time to detect front vehicle starting deceleration [s]
min_ego_accel_for_rss: -1.0 # ego's acceleration to calculate RSS distance [m/ss]
min_object_accel_for_rss: -1.0 # front obstacle's acceleration to calculate RSS distance [m/ss]
safe_distance_margin : 6.0 # This is also used as a stop margin [m]
terminal_safe_distance_margin : 3.0 # Stop margin at the goal. This value cannot exceed safe distance margin. [m]
cruise_safe_distance_margin : 0.5 # This is used as a cruise margin [m]
Copy link
Contributor

Choose a reason for hiding this comment

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

Due to the discontinuous distance when switching from CRUISE to STOP or STOP to CRUISE, we came across the issue where the velocity is sometimes not comfortable in the experiment.
With the current logic, It would be better to set parameters to the same values by default. Is it okay for you?

Copy link
Contributor Author

@ahmeddesokyebrahim ahmeddesokyebrahim Jul 24, 2024

Choose a reason for hiding this comment

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

Thanks for your review @takayuki5168 -san.

It would be better to set parameters to the same values by default.

Do you mean by this comment to set the default value of cruise_safe_distance_margin with 6.0 meters by default ?

if the answer is yes, so if this cruise_safe_distance_margin a bit bigger value than 0.5, (2.0 for example) that would solve the mentioned problem ?

Copy link
Contributor

Choose a reason for hiding this comment

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

What I meant is that safe_distance of the STOP and CRUISE should be totally the same (both 6.0) due to the discontinous distance explained above.
2.0 does not solve my problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks again @takayuki5168 -san for your comment.

Actually having the value of cruise_safe_distance_margin with 6.0 will turn into that the intended Dense Urban ODD scenarios for which we are proposing this change, they will still be failing with main Autoware !!

In addition, what will be then the significance of the parameters separation in obstacle_cruise_planner package implemented using autoware.universe PR if we are setting both parameters with the same value ?
The purpose of separating these two parameters is having the ability to assign different values for them tackling different driving situation.

If we confirm that the discontinuous distance when switching from CRUISE to STOP or STOP to CRUISE is leading to some behavior degradation and failing of other scenarios, then it would be better that you share more details about this problem and maybe we can work on adding a solution for it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ahmeddesokyebrahim
Sorry, but I don't have a detailed scenario.
Theoretically, during the CRUISE state, the stable target distance is cruise_safe_distance_margin, and after switching from CRUISE to STOP, the target distance becomes stop_safe_distance_margin. Therefore, if cruise_safe_distance_margin is smaller than stop_safe_distance_margin, the ego cannot keep the distance stop_safe_distance_margin when the ego stops.
That's why these distances have to be the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @takayuki5168 -san for your reply.

The development of this PR will be on-hold until further changes to be done in the scenario itself.
Afterwords, we can re-test again and check if we still need the parameters separation or not, and based on that we can check the discontinuity problem.

@ahmeddesokyebrahim
Copy link
Contributor Author

Closing this PR as the intended issue and scenario/s will be covered by this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Dense-Urban-ODD] Improve cruise planner safe distance when cruising a front npc
2 participants