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

PriorityQueue needs rework #1024

Open
rajatjain1997 opened this issue Feb 10, 2019 · 2 comments
Open

PriorityQueue needs rework #1024

rajatjain1997 opened this issue Feb 10, 2019 · 2 comments

Comments

@rajatjain1997
Copy link
Contributor

rajatjain1997 commented Feb 10, 2019

As pointed out by #919 and #941, A* search is behaving incorrectly. This is partly due to the way the A* implementation interfaces with the PriorityQueue (in utils.py). Due to how __contains__ is implemented in PriorityQueue, if the heuristic function passed to A* produces non-unique values for the same state (As in the case of #919), A* would re-visit the same state multiple times.

#829 also raises some concerns about the implementation of __getitem__ and __contains__ in PriorityQueue. While these methods seem similar, they are quite distinct. Using __getitem__ instead of __contains__ should fix A*. However, since both these functions are confused many times (#828), we should include at least some documentation and test cases to illustrate this use. Another solution to this confusion would be altering the roles of __getitem__ and __contains__ such that for a priority queue A and element x:

  • x in A returns whether a copy is present regardless of its function value

  • A[x] returns the function value of a copy if it is present in the queue, else throws a KeyError like other dicts in python.

@rajatjain1997 rajatjain1997 changed the title PriorityQueue needs rework A* search needs rework Feb 10, 2019
@rajatjain1997 rajatjain1997 changed the title A* search needs rework PriorityQueue needs rework Feb 10, 2019
@zeph1yr
Copy link

zeph1yr commented Feb 13, 2019

Is this issue resolved? If resolved, why is it still open?

@rajatjain1997
Copy link
Contributor Author

Yes. My PR works to resolve this issue and those I've mentioned in it. I think they'll be closed once the PR is merged.

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

No branches or pull requests

2 participants