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

Prediction Module for data dumper compatibility check #119

Closed
wants to merge 22 commits into from

Conversation

XHwind
Copy link
Contributor

@XHwind XHwind commented Aug 2, 2021

No description provided.

@XHwind XHwind added the enhancement New feature or request label Aug 2, 2021
@XHwind XHwind requested a review from DerrickXuNu August 2, 2021 16:49
@XHwind XHwind self-assigned this Aug 2, 2021
Copy link
Contributor

@DerrickXuNu DerrickXuNu left a comment

Choose a reason for hiding this comment

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

Please address the comments I added before merging.

@@ -58,6 +58,13 @@ def draw_trajetory_points(world, waypoints, z=0.25,
arrow_size=arrow_size,
color=color,
life_time=lt)
def draw_points(world, points, z, color=carla.Color(255, 102, 0), lt=0.06, size=0.05):
Copy link
Contributor

Choose a reason for hiding this comment

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

  • [ Please write docstring for this to tell users what is the meaning of every variable and what is the data type]

@@ -105,6 +106,9 @@ def __init__(self, vehicle, carla_map, config_yaml):
# collision checker
self._collision_check = CollisionChecker(
time_ahead=config_yaml['collision_time_ahead'])
# Warning!!
# hard coded!!!
Copy link
Contributor

Choose a reason for hiding this comment

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

  • [ Use todo/TODO to note the things you need to change later]

@@ -105,6 +106,9 @@ def __init__(self, vehicle, carla_map, config_yaml):
# collision checker
self._collision_check = CollisionChecker(
time_ahead=config_yaml['collision_time_ahead'])
# Warning!!
# hard coded!!!
self.prediction_manager = PredictionManager(int(4/0.05), int(3/0.05), 0.05)
Copy link
Contributor

Choose a reason for hiding this comment

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

You can pass the parameter fixed_delta_second from the yaml file here instead of hard coding it. Check behavior planner as an exmaple.

def angle_diff(x, y):
"""
Get the smallest angle difference between 2 angles: the angle from y to x.
Parameters
Copy link
Contributor

Choose a reason for hiding this comment

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

new line here



class TrajectoryData:
def __init__(self, l):
Copy link
Contributor

Choose a reason for hiding this comment

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

Remember to add docstring for every class in the future.

@@ -0,0 +1,274 @@
# -*- coding: utf-8 -*-
"""
Physcis-based trajectory prediction model
Copy link
Contributor

@DerrickXuNu DerrickXuNu Aug 2, 2021

Choose a reason for hiding this comment

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

Restructure this core script please. Rename it to physics_model, and put all functions that are not a member of any class to another script(suggested name: physics_utils). By this way, the code structure is more clear.

return np.linalg.norm(x - y)

assert dist(preds, gt) < 1e-10
return
Copy link
Contributor

Choose a reason for hiding this comment

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

no need to write return for such method.

@DerrickXuNu
Copy link
Contributor

Some general comment: add prediction module parameters in the yaml file so users could choose different model and set different params.

@DerrickXuNu DerrickXuNu deleted the feature/prediction branch April 27, 2023 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants