refactor: replace analytics shortest path algorithms with OD graph algorithms#903
refactor: replace analytics shortest path algorithms with OD graph algorithms#903
Conversation
This commit prepares the computeShortestPaths algorithm to replace the algorithms currently used for analytics. The main goal is to allow retrieving the paths (trainrun section IDs) in addition to the total spent time. Details: - Track all traversed trainrun sections in computeShortestPaths, to allow reconstructing the paths in the end of the function - Extend computeShortestPaths to return Map<nodeId, [cost, connections, trainrunSectionIds]> rather than Map<nodeId, [cost, connections]> - Adds connections count as a tiebreaker for paths with similar cost Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
This commit replaces ShortestTravelTimeSearch with the existing OD graph utilities (buildEdges, computeNeighbors, topoSort, computeShortestPaths). This removes the FilterService dependency from AnalyticsService, as visibility filtering is now handled upstream by getVisibleTrainruns. Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
This commit removes ShortestTravelTimeSearch and ShortestDistanceEdge, replaced with the OD graph utilities in the previous commit. Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
aiAdrian
left a comment
There was a problem hiding this comment.
Very nice work. 1st step towards the correct implementation is done!
- I just got, that for trainruns which start not at zero minute have a departure offset -> this means the traveltime is 0 at departure node -> dep at n -> travel time m -> arrival time n+m -> shortest travel time n+m
-> we have to discuss this
-> the rendering is not yet fine - but as good as it was :-) -> we have to discuss how to improve it later
THANK YOU VERY MUCH -> nice work !
| } | ||
| dist.set(neighbor, [alt, dist.get(vertex)[1] + connection]); | ||
| const newConnections = dist.get(vertex)[1] + connection; | ||
| // We use the connections count as a tiebreaker for paths with equal |
There was a problem hiding this comment.
any specific reason for this change?
There was a problem hiding this comment.
When I was trying to deduplicate the paths on screen (to resolve the 2. Some unavoidable duplications described in the PR), I first tried this basic tiebreaker, which in some case I had actually resolved the issue.
I later decided to keep it, as having some tiebreaker makes the final decision more explainable, and this one in particular felt meaningful to me, while remaining quite easy to implement and maintain, basically.
There was a problem hiding this comment.
I see, thanks, I think a comment to explain the usecase would be nice
I think this might be more complicated, since the available connections are different either if we start at |
| // Maps each reached nodeId to its arrival "convenience" vertex. | ||
| const destinations = new Map<number, Vertex>(); | ||
| // Maps each vertex to its best predecessor, for path reconstruction. | ||
| const prev = new Map<Vertex, Vertex>(); |
| /** | ||
| * Compute shortest paths from a departure node using the OD graph algorithm. | ||
| * When startTrainrunSectionId is set, the first hop is constrained to that | ||
| * section, and paths back to the departure node are excluded. |
There was a problem hiding this comment.
why are we excluding those?
This PR refactors the analytics shortest-path computation, to reuse the existing OD graph infrastructure (
buildEdges,computeNeighbors,topoSort,computeShortestPaths) instead of the standaloneShortestTravelTimeSearchalgorithm.Some additional explanations on the output
1. More parallel highlighted sections
In some
A->B->Ccases, the OD graph algorithm sometimes can rightfully find a path from A to C that does not follow the shortest path from A to B, because it now starts "counting the cost" at the departure time of the first trainrun section, rather than 00:00 all the time, as it did before:2. Some unavoidable duplications
The algorithm fails to use existing shortest subpaths as a tiebreaker for longer paths. So, in some cases, a long shortest path can highlight some sections that are parallel to a "shorter shortest path" (i.e. a shortest path to some node closer to the start node). This is a limitation that sometimes adds some noise to the final visual output. I did not find a simple enough way to mitigate it, though, but it certainly can be done later.
In the following example, both the path 1 (IC81) and path 2 (IR15) are highlighted from Bern to RTR. The path 1 makes a lot of sense, because it belongs to the actual shortest path from Lausanne (start node) to RTR. But both paths 1 and 2 can be used, in the shortest path from Lausanne to Langent., and here, the algorithm selects the path 2. And improved version could only highlight the path 1 in this very case.