AnishShr / autoware_mini_practice

Practice Sessions and Labs for the Autonomous Vehicles Project (Autoware mini)
MIT License
0 stars 0 forks source link

Practice 3 #2

Closed geopimik closed 7 months ago

geopimik commented 7 months ago

Please fix the following

  1. You are missing shebang line from the beginning of your follower node! Not sure how you ran it. https://github.com/AnishShr/autoware_mini_practice/blob/3c203454f6dc783e44df131b431797638f767bc9/practice_3/nodes/control/pure_pursuit_follower.py#L1
  2. You don't need to subscribe to /initialpose and use it in the follower. Having an initial pose is specific to the simulation. We need to place the ego vehicle somewhere so that the simulator can start from somewhere. In real world settings, the localizer provides the ego vehicle location, and we will get the location from the /localization/current_pose. We should rely only on that topic to make the follower more universal. After the initial pose is set the simulator substitutes the localizer and starts to publish the /localization/current_pose. And in real world settings we won't have the initial pose. So remove the initial pose from the follower node. https://github.com/AnishShr/autoware_mini_practice/blob/3c203454f6dc783e44df131b431797638f767bc9/practice_3/nodes/control/pure_pursuit_follower.py#L33
  3. Remove unnecessary TODO https://github.com/AnishShr/autoware_mini_practice/blob/3c203454f6dc783e44df131b431797638f767bc9/practice_3/nodes/control/pure_pursuit_follower.py#L41
  4. You are looping over the waypoints two times. Reorganize it so it is done only once.
  5. I would suggest assigning the following (see the code block below) together at the very end of the path_callback:
    • and before assigning, I would calculate distance_to_velocity_interpolator into the local variable.
    • Imagine there is a long path, and the path_callback would take 0.25 seconds to run. During that time, we have at least two cycles of current_pose_callback, and if we first update path_linesting and write it to the class variable, then we prepare distance_to_velocity_interpolator and assign it to the class variable, we can create a situation where in the current_pose_callback they are not in sync.
    • So preparing them into local variables and doing the assigning together and at the very end of the path_callback minimizes the risk of this happening
      self.path_linestring = path_linestring
      self.distance_to_velocity_interpolator = interp1d(distances, velocities, kind='linear')

Please answer

  1. Something looks a bit different in the formula, but interestingly it produces the same result as the formula in the practice materials, so maybe these changes kind of make it the same. Can you comment on that?
    • there should be arctan instead of arctan2
    • it should be division with ld

https://github.com/AnishShr/autoware_mini_practice/blob/3c203454f6dc783e44df131b431797638f767bc9/practice_3/nodes/control/pure_pursuit_follower.py#L88

  1. In task 6 in instructions point 4 fill_value and bounds_error are expected to be used. It shouldn't happen that we get distance out of the length limits of the path linestring when using shapely. But if it happens? What will your code currently do? https://github.com/AnishShr/autoware_mini_practice/blob/3c203454f6dc783e44df131b431797638f767bc9/practice_3/nodes/control/pure_pursuit_follower.py#L61
AnishShr commented 7 months ago
  1. The catkin_install_python takes care of the python version dependency and appends the shebag line while building the package. Should I add the shebang line on top of the controller node still?

  2. Removed the /initialpose subscription.

  3. Removed unnecessary TODO

  4. Reorganized the program structure to loop over the waypoints only once https://github.com/AnishShr/autoware_mini_practice/blob/709fa2c8c34e524dc4770b6a334911bac1b3dd6d/practice_3/nodes/control/pure_pursuit_follower.py#L37-L41

  5. computed the velocity and stored in the local variable. In the end of the path_callback, assigned them to class variables https://github.com/AnishShr/autoware_mini_practice/blob/709fa2c8c34e524dc4770b6a334911bac1b3dd6d/practice_3/nodes/control/pure_pursuit_follower.py#L55-L63

  6. Generally, arctan2 should hold all properties that arctan holds. In theory, arctan2 is used for ranges between -180 and 180, while arctan is used for ranges -90 to 90. source: stackoverflow_thread. And for ld, the lookahead distance would be the distance from ego-vehicle's current position in the path to the lookahead point in the trajectory. I believe this is what was supposed to be done. Or did I misunderstood your question?

  7. Added the fill_value and bounds_error values while computing the velocities fro distance. bounds_error will raise an error if the input distance is out of bounds from the limits fill_value will set the velocity value to be 0 for the points outside the bounds of distance inputs https://github.com/AnishShr/autoware_mini_practice/blob/709fa2c8c34e524dc4770b6a334911bac1b3dd6d/practice_3/nodes/control/pure_pursuit_follower.py#L55-L59

Latest commit: latest commit

geopimik commented 7 months ago
  1. OK - no need. Have not used catkin_install_python personally
  2. OK
  3. OK
  4. OK
  5. OK
  6. OK, if you use atan then there is division with ld in original formula, but it seems to produce the same thing as atan2(the formula without dividing with ld, ld). Anyway it seems correct, I was just a bit confused at first.
  7. bouds_error by default is true, but we don't want it to be raised, we just want to set the values to 0 if out of bounds distances are sent to interpolator.
AnishShr commented 7 months ago
  1. arctan2() has division with ld ,inside the paranthesis there are 2 values. https://github.com/AnishShr/autoware_mini_practice/blob/709fa2c8c34e524dc4770b6a334911bac1b3dd6d/practice_3/nodes/control/pure_pursuit_follower.py#L90

https://numpy.org/doc/stable/reference/generated/numpy.arctan2.html

I guess you were confused by how numpy provides the API for arctan and arctan2. It is slightly different, but eventually provides the same result.

geopimik commented 7 months ago
  1. One tiny detail, but no need to change anything https://github.com/AnishShr/autoware_mini_practice/blob/8dd232a77319ab8eb8a20a69bd2aceb8b6246b32/practice_3/nodes/control/pure_pursuit_follower.py#L55-L59

Shapely should not give a distance that is below 0 or above the length of the line string, but lets say that we change something there or it somehow gives a bit longer value. The bounds_error regulates what happens then:

It can be used both ways! Depends what you want to achieve, how important is to know about it etc... Currently I asked to not raise an error, but again it depends what we want to achieve... Important is to understand what is happening in the code.

OK