-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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 expanded node set to SpaceTime AStar #1183
base: master
Are you sure you want to change the base?
Add expanded node set to SpaceTime AStar #1183
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Overview
This PR improves the performance of the SpaceTime AStar algorithm by introducing an expanded node set to avoid duplicate expansions and adds timing information to measure planning runtime.
- Implements an expanded set and list to track visited nodes
- Adds a timer in the main function to track planning duration
- Updates the unit test to assert that the number of expanded nodes remains below a threshold
Reviewed Changes
File | Description |
---|---|
PathPlanning/TimeBasedPathPlanning/SpaceTimeAStar.py | Incorporates an expanded set to avoid duplicate node expansions and adds timer functionality |
tests/test_space_time_astar.py | Updates the test to assert a maximum bound on expanded nodes |
PathPlanning/TimeBasedPathPlanning/GridWithDynamicObstacles.py | Adds a hash method to the Position class for proper use in hashed collections |
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
return hash((self.position, self.time, self.heuristic)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hash method in Node includes the heuristic value, but eq only compares position and time. This inconsistency can lead to unexpected behavior in hashed collections; consider using only position and time in both methods.
return hash((self.position, self.time, self.heuristic)) | |
return hash((self.position, self.time)) |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SchwartzCode What do you think this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ayo, that is a nice catch! Go copilot 🎉
I removed self.heuristic
and added a note why that & cost are not included in __eq__
or __hash__
:
eecb5c8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add an explanation about this speed-up technique to the RST documentation? Something like the PR description plus a little more would be fine.
@AtsushiSakai added some information to the RST doc for this algorithm: Does that look good? |
Reference issue
What does this implement/fix?
This change speeds up the SpaceTime A* algorithm by adding an expanded set. The algorithm will not expand nodes that are already in that set. This greatly reduces the number of node expansions needed to find a path, since no duplicates are expanded. It also helps to reduce the amount of memory the algorithm uses.
Also adds a timer to the
SpaceTimeAStar.py
scriptBefore:
After:
When starting at (1, 11) in arrangement1
Additional information
CheckList