Skip to content

switch to a* algorithm for shortest path#199

Merged
robfitzgerald merged 2 commits intoNatLabRockies:mainfrom
erik-hasse:astar-shortest-path
May 1, 2023
Merged

switch to a* algorithm for shortest path#199
robfitzgerald merged 2 commits intoNatLabRockies:mainfrom
erik-hasse:astar-shortest-path

Conversation

@erik-hasse
Copy link
Copy Markdown

@erik-hasse erik-hasse commented Apr 26, 2023

This appears to have a slight slowdown for small networks, but a speedup for larger ones. In my very unscientific tests I got:

Denver ~25% slower
dijkstra: 5.41s
a*: 6.76s

Manhattan ~17% faster
dijkstra: 89.35s
a*: 73.78s

Closes #197

@erik-hasse erik-hasse force-pushed the astar-shortest-path branch from b22d55c to 7ac301d Compare April 26, 2023 20:33
@robfitzgerald
Copy link
Copy Markdown
Collaborator

robfitzgerald commented Apr 26, 2023

This appears to have a slight slowdown for small networks, but a speedup for larger ones

makes sense to me. we've added an extra haversine distance estimate to each link discovered in the shortest path algorithm; we've reduced the number of links we need to look at in the average search.

i'm curious how the overall vkt appears in a before/after. if that is very similar, i would feel comfortable to accept this without spending more time trying to verify to ourselves that the routes are still travel time optimal.

edit: don't know why i just said that, i'm going ahead and running that comparison now 🤦‍♂️

@robfitzgerald
Copy link
Copy Markdown
Collaborator

before: 82983.4 km
after: 93624.56 km

hmm, ok. i mean, i think we haven't ironed out all of the nondeterminism in hive, but, a 12% increase in VKT might be unexpected. i can't look more into this today, but, i think we should try and confirm for ourselves that we are still getting optimal path search results.

thanks for quickly whipping this PR, @erik-hasse!

Copy link
Copy Markdown
Collaborator

@robfitzgerald robfitzgerald left a comment

Choose a reason for hiding this comment

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

i double-checked my work and it turns out my A/B test wasn't correctly based on the same base. after fixing, i'm seeing < 0.01% change in VKT, and that's enough for me to feel good about the a* heuristic implementation. thanks for the help on this!

@robfitzgerald robfitzgerald merged commit 05add0f into NatLabRockies:main May 1, 2023
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.

use a* instead of dijkstra's for runtime improvement

2 participants