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

Add new follow task #15

Merged
merged 2 commits into from
Nov 25, 2024
Merged

Add new follow task #15

merged 2 commits into from
Nov 25, 2024

Conversation

MICH3LL3D
Copy link
Contributor

  • created a new following task
  • added appropriate steps in tasks.yaml
  • changed one line in wpt_env file (destination reached)
  • ignore all the right turn changes

Copy link
Collaborator

@gaodechen gaodechen left a comment

Choose a reason for hiding this comment

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

Thank you for implementing the new CarDreamer task carla_follow. This is awesome! Left a few comments to be addressed before we merge the PR into the main branch, including log and temporary files to be cleaned up, readability issues, and changes that might affect other tasks. Feel free to let Hanchu or me know if you got any blockers and questions. We appreciate your contribution :)

24.1 Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

the file is an empty pip log and can be removed

bin/tabulate Outdated
@@ -0,0 +1,8 @@
#!/home/dongx/miniconda3/envs/cardreamer/bin/python
Copy link
Collaborator

Choose a reason for hiding this comment

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

seems the script is not needed here. and the header contains a local path

Copy link
Collaborator

Choose a reason for hiding this comment

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

pyc files should be filtered out by gitignore

max_num_of_directions = len(self._config.lane_start_points)
if self._config.direction == max_num_of_directions:
self.random_num = random.randint(0, max_num_of_directions - 1) # random path
elif self._config.direction >= 0 and self._config.direction < max_num_of_directions:
Copy link
Collaborator

Choose a reason for hiding this comment

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

it seems that each direction values corresponds to specific meanings. it would be more readable to have enum constants defined in the utils

@@ -131,7 +131,8 @@ def reward(self):
return total_reward, info

def is_destination_reached(self):
return len(self.waypoints) <= 3
# return len(self.waypoints) <= 3
Copy link
Collaborator

Choose a reason for hiding this comment

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

wpt env is a base class that supports multiple different tasks like follow, right turn, left turn, and more. wondering if the change does not affect other tasks

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is fantastic!! Thank you for the contributions!! Just left a few comments to be addressed before we merge the changes.

@HanchuZhou HanchuZhou merged commit 2ed18bc into ucd-dare:master Nov 25, 2024
0 of 2 checks passed
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