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

Added A* search algorithm implementation #8

Merged
merged 27 commits into from
Mar 10, 2019

Conversation

bartoszpankratz
Copy link
Collaborator

No description provided.

src/a_star.jl Outdated Show resolved Hide resolved
@pszufe
Copy link
Owner

pszufe commented Mar 5, 2019

@bartoszpankratz - build failed:

ERROR: LoadError: ArgumentError: Package OpenStreetMapX does not have DataStructures in its dependencies:

Do you know how to add the dependency or need help?

@bartoszpankratz
Copy link
Collaborator Author

@bartoszpankratz - build failed:

ERROR: LoadError: ArgumentError: Package OpenStreetMapX does not have DataStructures in its dependencies:

Do you know how to add the dependency or need help?

Nope, any help will be very appreciated.

@pszufe
Copy link
Owner

pszufe commented Mar 8, 2019

@bartoszpankratz I have fixed the dependencies.
However what is missing:

  1. add the a_star routing to shortest_route and fastest_route methods. a_star should be used by default. I propose adding a new parameter to those methods ; routing = :astar and the user should be able to change ; routing = :dijkstra if they wanted the classic Dijkstra.
  2. Once (1) is done add the following tests in the runtests.jl file:
     @test shortest_route(m, pointA, pointB; routing = :astar) == shortest_route(m, pointA, pointB; routing = :dijkstra)
     @test fastest_route(m, pointA, pointB; routing = :astar) == fastest_route(m, pointA, pointB; routing = :dijkstra)
    
    I assume that those tests should pass.

@bartoszpankratz
Copy link
Collaborator Author

Done

@pszufe
Copy link
Owner

pszufe commented Mar 9, 2019

@bartoszpankratz tests now are failing for the fastest_route
we need to adjust the heuristic function for a_start to calculate in this case the shortest theoretical travel time. Probably now it is still calculating distance - so for fastest_route we it needs to be divided by the travel speed.

@bartoszpankratz
Copy link
Collaborator Author

@pszufe I know it. An it was one of the reasons why I insisting on keeping the zeros heuristic during the phone call. It is the only universal one, and If you need something more sophisticated declaring new one is easy and absolutely basic.
Straight Line heurisic is proper only for the shortest path searching, for the driving time you need to create another large matrix (speeds matrix) and it still will be sometimes unnefective (for segments faster than average speed the heuristic value might be larger than the value of the route, so algorithm will return unoptimal path).

@bartoszpankratz
Copy link
Collaborator Author

I have modified this heuristic, now it is dividing not by not average but maximum possible speed. IMHO it is quite unnatural but works - it will never overestimate the cost of the route. I guess case is done.

@bkamins
Copy link
Collaborator

bkamins commented Mar 10, 2019

This PR is becoming unmanageably large (almost 300 LOC).
@bartoszpankratz - is it possible to split it into two parts:

  • a minimal working solution (but implemented right) - this PR
  • additional features - next PRs (probably we can already open issues for these features not to forget them)

@pszufe pszufe merged commit 31fb4e1 into pszufe:master Mar 10, 2019
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