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

Argoverse waypoint improvements in junctions #1944

Merged
merged 5 commits into from
Apr 17, 2023

Conversation

saulfield
Copy link
Contributor

This addresses #1926, specifically the 3rd point raised in the issue. We now keep track of the last junction lane used for waypoints, to prevent erratic behaviour when the vehicle is on top of multiple lanes.

This also addresses #1927, by generating waypoints for all paths leading into the current junction lane:

argoverse-junctions1.mp4

@Adaickalavan
Copy link
Member

Wondering whether it would be possible to slightly tighten the potential waypoint paths which now appear a little too loose?

Screenshot from 2023-04-06 15-52-35

@saulfield
Copy link
Contributor Author

The problem is that there is no road connectivity between that road and where the ego is, so there is no valid path leading back to those waypoints, though they are still part of the junction. Would it be suitable to filter out those waypoints earlier on so that they aren't so far away?

CHANGELOG.md Outdated Show resolved Hide resolved
smarts/core/argoverse_map.py Outdated Show resolved Hide resolved
@@ -881,7 +881,7 @@ def closest_lanepoints(
k=maximum_count,
filter_composites=True,
)
return [l_lps[0].lp for l_lps in linked_lanepoints]
return [l_lps.lp for l_lps in linked_lanepoints[0]]
Copy link
Collaborator

@Gamenot Gamenot Apr 6, 2023

Choose a reason for hiding this comment

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

Before
+---+---+---+
|p1 |p2 |p3 |
+---+---+---+
| x | x | x |
+---+---+---+
|   |   |   |
+---+---+---+
After
+---+---+---+
|p1 |p2 |p3 |
+---+---+---+
| x |   |   |
+---+---+---+
| x |   |   |
+---+---+---+

Am I mistaken in thinking that now all the lps will be related to the first pose?

Maybe the method is not doing what it says it should be. It gives a maximum_count parameter but currently only gives a single lanepoint per pose. It does use maximum_count to figure out the maximum lanepoints to consider for the one lanepoint it gives per pose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. I didn't even realize that this takes in multiple poses. I'm a bit confused by the intention of this function, considering it seems to only ever be called with 1 pose. The original definition only took the first lanepoint of each set of 10 lanepoints for each pose. I think I may just change it to take a single pose and leave it as-is.

smarts/core/argoverse_map.py Outdated Show resolved Hide resolved
return angle

# Filter out any waypoints that are behind the vehicle
wps = [wp for wp in path if _angle_between(pose, wp) < np.pi / 1.5]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This might not work because the vehicle could get turned around and lose out on seeing waypoints that are ahead of where it should go because it is turned around.

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

Considering @Adaickalavan's comment on the junction waypoints I am not entirely certain what should be done.

  • Filter branches by using the closest similarly near-oriented point's orientation to cull other branches
  • Cut off branches which have the nearest point far away and ends oriented away.
  • Leave roughly as is

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are now filtering based on distance to first lanepoint.

@Adaickalavan
Copy link
Member

The problem is that there is no road connectivity between that road and where the ego is, so there is no valid path leading back to those waypoints, though they are still part of the junction. Would it be suitable to filter out those waypoints earlier on so that they aren't so far away?

  • Yes, perhaps we could keep the right-turn path until this point in time (shown below). This would ensure the path options are not too strict.
  • After this point in time, when the right-turn path becomes too far away, we could filter it out if possible. This would ensure the path options are not too loose.

Screenshot from 2023-04-06 17-05-02

I just realised that the same issue of far-away waypoints occur in the left-turn path too.

Screenshot from 2023-04-06 17-13-29

@saulfield
Copy link
Contributor Author

I've made some changes based on the feedback.

  • Filtered out waypoint paths that are out of range
  • Removed the caching, and instead generated (unique) paths for all junctions lanes that the vehicle is on

This should now give all possible paths, and relevant paths should not suddenly disappear. However, sometimes new paths can suddenly appear if the vehicle drives onto a new lane. I think this is fine, since an agent can determine for itself which waypoints to follow.

See the video below for a demo:

argoverse-junctions3.mp4

@saulfield saulfield requested a review from Gamenot April 12, 2023 19:07
Copy link
Collaborator

@Gamenot Gamenot left a comment

Choose a reason for hiding this comment

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

Only one concern and this can be merged.

smarts/core/argoverse_map.py Show resolved Hide resolved
@saulfield saulfield merged commit 4d8e052 into master Apr 17, 2023
@saulfield saulfield deleted the saul/argoverse-waypoints branch April 17, 2023 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants