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

Use g value instead of f value to create sucessors in A* #268

Merged
merged 2 commits into from
Aug 1, 2022

Conversation

knoellle
Copy link
Contributor

@knoellle knoellle commented Jun 8, 2022

This pull request fixes an error in the A* implementation and adds two separate test cases to prevent the issue I was facing in the future.

Fixes #267

Details

The A* implementation previously used these formulas to calculate the new g and f values for a node:

successor.g = previous.f + step_cost
successor.f = previous.f + step_cost + distance_to_end

The correct equations however use the previous.g value instead. See here, here, or here for reference.

g is the cost to reach the node from the start, f is g plus a heuristic for the remaining cost to reach the destination.
Using the cost + heuristic to calculate the cost for the next node does not make sense, the new f value would then contain the heuristic for the distance between the two nodes twice, resulting in a value that increases with the number of nodes in a path instead of the distance. This is why I added the more generic testcase.

I also changed the way the cost is passed to the add_successor function. Instead of passing q.g + step_cost, now only the step cost is passed. This is because q (and thus also q.g) is already passed to the function and the I think the new cost should conceptually be calculated inside add_successor instead of the call-site.

@thebracket
Copy link
Collaborator

I'll try and get this merged this week. Thanks!

@thebracket thebracket merged commit e8c4fdf into amethyst:master Aug 1, 2022
thebracket added a commit that referenced this pull request Oct 4, 2022
…culations, the astar example actually finds a path now (updates logic from #268)
@knoellle knoellle deleted the fix branch October 28, 2022 16:02
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.

A* Implementation Takes Expensive Shortcuts
2 participants