-
Notifications
You must be signed in to change notification settings - Fork 18
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
[WIP] refactor routing algorithms #58
base: master
Are you sure you want to change the base?
Conversation
One thing that hasn't made sense to me yet: why are the nodes in the road network (constructed by Dict{Int64,KeyVertex{Int64}} with 92 entries:
575440648 => KeyVertex{Int64}(1,575440648)
1053454370 => KeyVertex{Int64}(2,1053454370)
2 => KeyVertex{Int64}(3,2)
⋮ => ⋮ and not Dict{Int64,KeyVertex{Int64}} with 92 entries:
575440648 => 1
1053454370 => 2
2 => 3
⋮ => ⋮ or
|
Okay, I think I've gotten to the root of it: the My proposal is that we introduce an type Network
g # Graph object
v::Vector{Int} # OSM IDs
index::Dict{Int,Int} # (OSM ID) => index (for the reverse mapping)
w::Vector{Float64} # Edge weights
class::Vector{Int} # Road class of each edge
end This makes it easier to reason about the Network, without having to second-guess what we're working with -- It's unclear why the nodes shouldn't be the OSM IDs themselves, and the additional indices (in
and I think a natural choice should be: |
Yes, I think the reasoning behind |
Yeah, I've gone through the code (replacing everything), and the only place where it was needed so far was in I do think that it's cleaner to have a |
Ok, if there's a cleaner way to do it, then that's cool with me. And don't worry about delays, this won't add any functionality to me so I'm in no rush. By the way, I've added you as a collaborator to the repository, since you've been doing so much work with the project and I haven't been all that great at keeping up with your pace lately. Thanks for all your help so far! |
related: JuliaAttic/OldGraphs.jl#160 |
to use
hasparent
(to be introduced in Graphs.jl); not ready for merge yet!