Re-costing A Path #2425
Re-costing A Path #2425
Conversation
…so taht we have all the costing information available
src/sif/util.cc
Outdated
| // get the previous edges node | ||
| node = edge ? reader.nodeinfo(edge->endnode(), tile) : nullptr; | ||
| if (edge && !node) { | ||
| throw std::runtime_error("Node cannot be found"); |
There was a problem hiding this comment.
Optional: these unexpected errors should not be caught by the caller, should bubble up to become a 500 error.
valhalla/baldr/time_info.h
Outdated
| } | ||
|
|
||
| /** | ||
| * Helper function to initialize the object from a location. Uses the graph to |
There was a problem hiding this comment.
s/from a location/from a datetime
| @@ -155,6 +155,7 @@ const std::unordered_map<unsigned, std::string> OSRM_ERRORS_CODES{ | |||
| {124, R"({"code":"InvalidOptions","message":"Options are invalid."})"}, | |||
| {125, R"({"code":"InvalidOptions","message":"Options are invalid."})"}, | |||
| {126, R"({"code":"InvalidOptions","message":"Options are invalid."})"}, | |||
| {127, R"({"code":"InvalidOptions","message":"Options are invalid."})"}, | |||
There was a problem hiding this comment.
Can you add a more recostings-specific error message here?
There was a problem hiding this comment.
i think we should save that for another pr. at the moment, to remain osrm compatible we decided to, as much as we could, use messages that existed in that project. maybe its time to open up a discussion around whether that really makes sense anymore. i hope you are cool with doing that on another issue/PR though?
* use constrained time of day by default for any non-time dependent speed look up * didnt need this change * i didnt touch this code but somehow its linted differently.. * routes bench adapt to costing refactor
| // TODO: test restrictions | ||
| // we need to move to the next node which has the elapsed time at the end of the edge | ||
| ++elapsed_itr; | ||
| EXPECT_NEAR(elapsed_itr->cost().elapsed_cost().seconds(), label.cost().secs, .1); |
There was a problem hiding this comment.
here we test only that the transition.secs and elapsed.secs are the same between route and recost, we should also test that the costs are the same
| // TODO: There's a difference in the way a route is costed in reverse vs forward this could be | ||
| // a result of FormPath in timedep_reverse or it could be a result of the difference between | ||
| // TransitionCost and TransitionCostReverse. When that is resolved we can enable these tests | ||
| return; |
There was a problem hiding this comment.
we need to get to the bottom of this, but in a separate issue/pr
Re-costing a path is an interesting concept. It can be useful when you have a shortcut in your path but for proper guidance you dont want to return a route with shortcuts, so you recover them but now the edge labels from the shortcut path and their costs need to be divided up. A recosting method would allow recovering the costs for the non-shortcut version of the path.
Re-costing could also be useful in determining the relative value of a certain path amongst differently configured costing models. For example one could change the start
date_timeof a given path and see how it fluctuates over the course of some period of time. Or you could even change something likeuse_hillsto see if hilliness is a major contributor to cost along a given path.Finally recosting can and will be used in time dependent bidirectional a*. One of the main issues with that refactoring is that you need to make a guess at the time at one end of the search. After the search is completed you need to update your guess to the real measured time of arrival/departure. To do this we can simply recost the final path to get the final time.
EDIT:
I ran RAD on 14k random auto routes of various lengths. Diffing for everything from the directions output (time/distance/narrative) there were a total of 63 differences. When removing time/distance from the output (because many of them were off by a single second) there were 0 differences.