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

chore(static_obstacle_avoidance): make a certain scenario succeed #1024

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sasakisasaki
Copy link

@sasakisasaki sasakisasaki commented Jun 11, 2024

Description

This PR makes a certain scenario succeed, but shows unexpected behavior as the provided video below.

(In advance discussion is performed with @brkay54 in the software working group meeting held on 0:00 (JST) 12nd June 2024).

Related links

autowarefoundation/autoware.universe#7485

Tests performed

On the scenario simulator as the attached video.
behavior

Notes for reviewers

The tuned parameter made the situation better, while the car oscillates wider gradually after the avoidance.

Interface changes

No interface changes, but this change is applied for all components which are using the static obstacle avoidance.

Effects on system behavior

Critical. Because merging this PR changes the behavior of car.

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.

@sasakisasaki sasakisasaki self-assigned this Jun 11, 2024
@sasakisasaki sasakisasaki changed the title chore(): Make a certain scenario succeed chore(static_obstacle_avoidance): Make a certain scenario succeed Jun 11, 2024
@sasakisasaki sasakisasaki changed the title chore(static_obstacle_avoidance): Make a certain scenario succeed chore(static_obstacle_avoidance): make a certain scenario succeed Jun 11, 2024
@sasakisasaki
Copy link
Author

Sorry, attaching video file on the description is kind of hard to see. Let me attach the video file here too for finding and checking easily!

Screencast.from.2024.06.11.18.49.40.webm

@sasakisasaki
Copy link
Author

@brkay54 I found the reason why the oscillation happens. I was using following command when running the scenario not to cause heavy resource load by using global_frame_rate:=5 (default is 30). It seems, using low frequency causes error accumulated control. After removing the option, the behavior became better as the attached video.

ros2 launch scenario_test_runner scenario_test_runner.launch.py \
  architecture_type:=awf/universe \
  record:=false \
  scenario:=$SCENARIO_YML \
  sensor_model:=sample_sensor_kit \
  vehicle_model:=sample_vehicle \
  global_frame_rate:=5
Screencast.from.2024.06.14.18.32.50.webm

@brkay54
Copy link
Member

brkay54 commented Jun 25, 2024

Sorry, @sasakisasaki -san for my late reply. Also, @ahmeddesokyebrahim proposed a similar change for these parameters if I remember correctly. As I understood, the problem is avoidance module does not select objects as avoidable for some cases.

@ahmeddesokyebrahim Could you check these parameters? As I remember, also we discussed the change same parameters. If this configuration also solves your scenario, we can merge it IMO.

@sasakisasaki
Copy link
Author

sasakisasaki commented Jun 26, 2024

Thank you @brkay54 for your confirmation. I'm going to do double check if this fix still works on the latest Autoware. And also I'll check if this fix does not cause any side effect for the other modules by asking my colleagues by next meeting on 2nd July.

@sasakisasaki
Copy link
Author

@shmpwk san, We want to have your opinion 🙏 . We are now doing feasibility study to adapt to some challenging scenarios by changing parameters. As we encountered an unexpected behavior by changing some parameters, I guess it might be better to use the non-main branch for the investigation/study purpose. If we want to have a branch for the purpose, which way looks better for us? Any your proposals are highly appreciated 🙏 . Thank you very much in advance!

@sasakisasaki
Copy link
Author

sasakisasaki commented Jul 2, 2024

My idea for now is (sorry for being still draft ideas),

  • Use forked repository
  • Use this repository and use study purpose branch
  • Use this repository and use main branch (w/ tag)
  • (anything else?)

@ahmeddesokyebrahim
Copy link
Contributor

Thanks @sasakisasaki for this PR.
Reflecting the discussion that we have earlier, I was working on the scenario UC-NTR-002-0001 where ego vehicle was not able to overtake parking NPC.
After investigation, I found lower the parameter hard_margin_for_parked_vehicle to 0.2 helped in triggering the avoidance module and successfully overtaking the parked NPC. You may consider testing this scenario as well with your changes to take the advantage of having more one scenario passing with your change.

cc: @brkay54

@shmpwk
Copy link
Contributor

shmpwk commented Jul 4, 2024

@sasakisasaki
Sorry I missed your mentioning. We usually test tuning with forking repository.

@sasakisasaki
Copy link
Author

@shmpwk Thank you for sharing the information. I understand!

@sasakisasaki
Copy link
Author

@ahmeddesokyebrahim Thank you for providing the information which works in such the scenarios! Actually I also had the similar observation when changing the margins (honestly, I tried many changes and not sure which is the effective ones). After trying some tuning on my environment, I'll put the update here by tomorrow (7th July).

@sasakisasaki
Copy link
Author

@ahmeddesokyebrahim Perhaps I'm using something different combination of the parameters. I tried your proposal hard_margin_for_parked_vehicle=0.2 for vehicle, but I could not observe improvement on my local environment. Let me do double check if I'm doing something different 🙏 . I'm now using the Autoware whose version is that of 25th June 2024. First of all I'll check the possible changes on my local machine.

Again, thank you for sharing your ideas!

@brkay54
Copy link
Member

brkay54 commented Jul 8, 2024

@sasakisasaki -san, hi, I want to clarify something. I set several scenarios in the parent issue, which also include the problem of not avoiding parked vehicles. Could you fine-tune the package parameters to trigger static_obstacle_avoidance for all cases? Also, when proposing your changes, could you please explain and support the reasons for the parameter adjustments? Please also consider the other road users and try similar scenarios for them too (e.g. bicycles and motorcycles)

@sasakisasaki
Copy link
Author

@brkay54 Thank you for your support and having the online session today. Now I understood what is the next actions for me: focus on the static obstacle case and find the parameters which makes the scenario succeed 👍 .

Copy link

stale bot commented Sep 7, 2024

This pull request has been automatically marked as stale because it has not had recent activity.

@stale stale bot added the status:stale label Sep 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

4 participants