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

New A star algorithm pr #391

Merged
merged 18 commits into from Sep 20, 2020
Merged

New A star algorithm pr #391

merged 18 commits into from Sep 20, 2020

Conversation

weicent
Copy link
Contributor

@weicent weicent commented Sep 1, 2020

  1. rewrite A star algorithm, improved efficiency by searching the path from two side simultaneously (i.e. from start to goal + from goal to start)

  2. generate obstacles map randomly, the number of obstacle is adjustable

Find a path:
FindPath
FindPath2

No path to the goal, indicate the boundary confine robot or goal:
NoPath
NoPath2

@codecov
Copy link

codecov bot commented Sep 1, 2020

Codecov Report

Merging #391 into master will decrease coverage by 0.07%.
The diff coverage is 86.86%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #391      +/-   ##
==========================================
- Coverage   89.79%   89.72%   -0.08%     
==========================================
  Files         127      129       +2     
  Lines        9186     9422     +236     
==========================================
+ Hits         8249     8454     +205     
- Misses        937      968      +31     
Impacted Files Coverage Δ
tests/test_a_star_searching_two_side.py 73.68% <73.68%> (ø)
...thPlanning/AStar/A_Star_searching_from_two_side.py 88.01% <88.01%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff3ad5b...417271e. Read the comment docs.

@AtsushiSakai
Copy link
Owner

Thank you for PR. Before review, could you please add a test file for your code?

@weicent
Copy link
Contributor Author

weicent commented Sep 3, 2020

Thank you for PR. Before review, could you please add a test file for your code?

Hi Atsushi, thanks for your reply. may I ask what is 'test file'? I'm a newbie in github, it will be helpful if you can give me some hint, thanks in advance!

@sarimmehdi
Copy link
Contributor

@weicent
Copy link
Contributor Author

weicent commented Sep 4, 2020

@weicent use this as reference: https://github.com/AtsushiSakai/PythonRobotics/blob/master/tests/test_a_star.py

Thanks for let me know this, it helps a lot! The test file is added now!

@lgtm-com
Copy link

lgtm-com bot commented Sep 4, 2020

This pull request introduces 1 alert when merging eaf5833 into 46bdd2a - view on LGTM.com

new alerts:

  • 1 for Variable defined multiple times

@weicent
Copy link
Contributor Author

weicent commented Sep 4, 2020

Hi Atsushi, i'm sorry for bother you with so much commits.

It seems that i was trapped with the CI test (together with the dwa_pr, this is my first time to create pull request in github), sorry for the inconvenience for you.

Could you please give me some hint for how to fix the issue? the codecov/patch just indicate some lines are not covered in the test and i don't kown what to do with it.

Thank you in advance.

@AtsushiSakai
Copy link
Owner

@sarimmehdi Thank you for your support : ).

@weicent Thank you for your hard work. No problem, you can ignore the code cov error, because you added a new file.
Let's start reviewing DWA PR first. After that I will check this PR. Thanks again.

@AtsushiSakai
Copy link
Owner

@weicent Hi. Before code review, could you please merger master code to this PR branch?

@weicent
Copy link
Contributor Author

weicent commented Sep 13, 2020

@AtsushiSakai Hi, i have made the rebase for the branch, please consider my pr, thanks in advance!

Copy link
Owner

@AtsushiSakai AtsushiSakai 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 again. I have some comments. PTAL.

self.parent = parent
self.coordinate = coordinate

def setg(self, value):
Copy link
Owner

Choose a reason for hiding this comment

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

I think these setter functions are not needed for understanding algorithm. You can just access these fields.

return boundary


def border_line(closed, obs):
Copy link
Owner

Choose a reason for hiding this comment

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

get_border_line is better?

plt.pause(0.0001)


def node_to_coor(node_list):
Copy link
Owner

Choose a reason for hiding this comment

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

node_to_coordinate is better?

@@ -0,0 +1,348 @@
'''
Copy link
Owner

Choose a reason for hiding this comment

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

Could you please change all ''' to """ ?

# convert node list into coordinate list and array
goa_cor_list, goa_cor_array = node_to_coor(goal_close)

if goal_open == []: # no path condition
Copy link
Owner

Choose a reason for hiding this comment

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

Same upper comment.

og_intersect = \
[coor for coor in org_cor_list if coor in goa_cor_list]

if og_intersect != []: # a path is find, og_intersect!=[]
Copy link
Owner

Choose a reason for hiding this comment

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

same upper comment.

except Break as success:
success.print_info()
except NoPath as fail:
if origin_open == []:
Copy link
Owner

Choose a reason for hiding this comment

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

same upper comment.

else:
# else goal confined, find border for goal
border = border_line(goa_cor_list, obst)
if org_cor_list == []:
Copy link
Owner

Choose a reason for hiding this comment

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

same upper comment.

# and search ended with close_list=[]
org_cor_list.append(start)
org_cor_array = np.array(org_cor_list)
if goa_cor_list == []:
Copy link
Owner

Choose a reason for hiding this comment

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

same upper comment.

@weicent
Copy link
Contributor Author

weicent commented Sep 15, 2020

@AtsushiSakai Thanks for your patient reply! I have modified the code as your suggestion! Please let me know if you have any further comments!

Copy link
Owner

@AtsushiSakai AtsushiSakai 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 again. This is the last review. PTAL.

if show_animation: # draw the path
plt.plot(path[:, 0], path[:, 1], '-r')
plt.title('Robot Arrived', size=20, loc='center')
plt.show()
Copy link
Owner

Choose a reason for hiding this comment

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

Could you please add plt.pause(0.01) before plt.show()? In my environment, the final red line does not show without it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure!

return stop_loop


def searching_control(start, end, bound, obstacle):
Copy link
Owner

Choose a reason for hiding this comment

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

Can you make this function to return x and y position lists of the final path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your suggestion! its a good idea to return path coordinate list in searching control. I have modified as you suggested, the main() function will print path list if show_animation == False now.
Thanks again for your patient review!

Copy link
Owner

@AtsushiSakai AtsushiSakai left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for great PR!!.

@AtsushiSakai AtsushiSakai merged commit 43ab313 into AtsushiSakai:master Sep 20, 2020
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.

None yet

3 participants