-
Notifications
You must be signed in to change notification settings - Fork 5
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
support for algorithms with multiple trees/routes #189
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks good! There's a key error in the notebook but that should be a quick fix
"execution_count": 18, | ||
"metadata": {}, | ||
"output_type": "execute_result" | ||
"ename": "KeyError", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like there's one more place to update in the notebook to use r["route"].
Hmm actually I'm hitting an error when running this on a tomtom config:
The config is set up to use |
Interesting. I did mess with (and simplify) the handling of edge vs vertex oriented queries and looks like I messed up the logic. I'll look into it when I come in, which will be closer to 11 today |
@nreinicke found it. SearchApp::run_edge_oriented now wraps SearchApp::run_vertex_oriented, and i was attempting to inject the missing parameters into the query to allow for this. but instead of injecting the VertexIds, I injected the coordinates (origin_x|y). |
i need to double-check this again. i may have forgotten to slap the original edge pair back on the result for edge-based searches. |
ok! now fixed^2, should be good, tested w/ vertex-oriented and edge-oriented configs. |
sweet! just tested again and everything seems to be working great! |
this PR begins the work to support search algorithms which return more than one tree and/or route. a quick review on those possibilities:
in this PR, the SearchApp and CompassApp are rewritten to support these cases, which spilled into a few other concerns:
null
if no path,{}
(an object) for one path, or[{}]
(an array of objects) for multiple paths