Skip to content

i found another typo and problem greedy_best_first #8770

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

Closed
JadeKim042386 opened this issue May 26, 2023 · 0 comments · Fixed by #8775
Closed

i found another typo and problem greedy_best_first #8770

JadeKim042386 opened this issue May 26, 2023 · 0 comments · Fixed by #8775
Labels
awaiting triage Awaiting triage from a maintainer

Comments

@JadeKim042386
Copy link
Contributor

JadeKim042386 commented May 26, 2023

What would you like to share?

1. Typo

self.target.pos_y and self.target.pos_x are reversed.

successors.append(
Node(
pos_x,
pos_y,
self.target.pos_y,
self.target.pos_x,
parent.g_cost + 1,
parent,
)
)

parameter of Node is below

def __init__(
self,
pos_x: int,
pos_y: int,
goal_x: int,
goal_y: int,
g_cost: float,
parent: Node | None,
):

The reason why it worked well is that self.target.pos_y(6) and self.target.pos_x(6) are the same.

2. Class equality

__eq__ is not implement in Node class.

class Node:
"""
>>> k = Node(0, 0, 4, 5, 0, None)
>>> k.calculate_heuristic()
9
>>> n = Node(1, 4, 3, 4, 2, None)
>>> n.calculate_heuristic()
2
>>> l = [k, n]
>>> n == l[0]
False
>>> l.sort()
>>> n == l[0]
True
"""
def __init__(self, pos_x, pos_y, goal_x, goal_y, g_cost, parent):
self.pos_x = pos_x
self.pos_y = pos_y
self.pos = (pos_y, pos_x)
self.goal_x = goal_x
self.goal_y = goal_y
self.g_cost = g_cost
self.parent = parent
self.f_cost = self.calculate_heuristic()
def calculate_heuristic(self) -> float:
"""
The heuristic here is the Manhattan Distance
Could elaborate to offer more than one choice
"""
dy = abs(self.pos_x - self.goal_x)
dx = abs(self.pos_y - self.goal_y)
return dx + dy
def __lt__(self, other) -> bool:
return self.f_cost < other.f_cost

so the below child_node not in self.open_nodes or child_node not in self.open_nodes did't work.

if child_node not in self.open_nodes:
self.open_nodes.append(child_node)
else:
# retrieve the best current path
better_node = self.open_nodes.pop(self.open_nodes.index(child_node))
if child_node.g_cost < better_node.g_cost:
self.open_nodes.append(child_node)
else:
self.open_nodes.append(better_node)

3. node in self.open_nodes is always better_node

How can a previous path cost more than the next one? A node that has already entered self.open_nodes is always less expensive than the next node.
So I checked that it works well even if I simply modify it as below.

if child_node not in self.open_nodes:
    self.open_nodes.append(child_node)
# delete else

Additional information

I think sort() and pop(0) are unnecessary. I think it's rather to increase the complexity of time.

@JadeKim042386 JadeKim042386 added the awaiting triage Awaiting triage from a maintainer label May 26, 2023
JadeKim042386 added a commit to JadeKim042386/Python that referenced this issue May 26, 2023
@JadeKim042386 JadeKim042386 changed the title i found another typo in greedy_best_first i found another typo and others in greedy_best_first May 27, 2023
@JadeKim042386 JadeKim042386 changed the title i found another typo and others in greedy_best_first i found another typo and problem greedy_best_first May 27, 2023
JadeKim042386 added a commit to JadeKim042386/Python that referenced this issue May 27, 2023
JadeKim042386 added a commit to JadeKim042386/Python that referenced this issue May 27, 2023
JadeKim042386 added a commit to JadeKim042386/Python that referenced this issue May 27, 2023
- node in self.open_nodes is always better node
TheAlgorithms#8770
JadeKim042386 added a commit to JadeKim042386/Python that referenced this issue May 27, 2023
JadeKim042386 added a commit to JadeKim042386/Python that referenced this issue May 27, 2023
JadeKim042386 added a commit to JadeKim042386/Python that referenced this issue May 27, 2023
- node in self.open_nodes is always better node
TheAlgorithms#8770
JadeKim042386 added a commit to JadeKim042386/Python that referenced this issue Aug 15, 2023
JadeKim042386 added a commit to JadeKim042386/Python that referenced this issue Aug 15, 2023
JadeKim042386 added a commit to JadeKim042386/Python that referenced this issue Aug 15, 2023
- node in self.open_nodes is always better node
TheAlgorithms#8770
tianyizheng02 added a commit that referenced this issue Aug 15, 2023
* fix: typo
#8770

* refactor: delete unnecessary continue

* add test grids

* fix: add \_\_eq\_\_ in Node class
#8770

* fix: delete unnecessary code
- node in self.open_nodes is always better node
#8770

* fix: docstring

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix: docstring max length

* refactor: get the successors using a list comprehension

* Apply suggestions from code review

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Tianyi Zheng <tianyizheng02@gmail.com>
sedatguzelsemme pushed a commit to sedatguzelsemme/Python that referenced this issue Sep 15, 2024
* fix: typo
TheAlgorithms#8770

* refactor: delete unnecessary continue

* add test grids

* fix: add \_\_eq\_\_ in Node class
TheAlgorithms#8770

* fix: delete unnecessary code
- node in self.open_nodes is always better node
TheAlgorithms#8770

* fix: docstring

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix: docstring max length

* refactor: get the successors using a list comprehension

* Apply suggestions from code review

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Tianyi Zheng <tianyizheng02@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting triage Awaiting triage from a maintainer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant