matsim-org / matsim-libs

Multi-Agent Transport Simulation
www.matsim.org
477 stars 445 forks source link

Timing inconsistencies in VrpPaths (for diversion) #1743

Closed sebhoerl closed 2 years ago

sebhoerl commented 2 years ago

I'm currently porting the dispatcher from Alonso-Mora et al. to MATSim. When implementing the dispatcher, I noticed that some routing capabilities of dvrp provide inconsistent predictions of arrival times, even under strict free flow conditions and even when using the QSimFreeFlowTravelTime estimator. The code for the unit test can be found at https://github.com/matsim-org/matsim-libs/commit/3e4ad4a9f8829dec865dd5bd34af545215fb7deb.

The experiment in the unit test is as follows: I construct a very simple scenario with a straight line of 8 links with length 1000 m and an odd free flow speed, to make sure that the QSimFreeFlowTravelTime works properly and rounding does not have an influence. I then create one DVRP vehicle at L0 and add one task to the schedule, which makes it drive to L8. Every 10 seconds, I obtain the next diversion point for the vehicle from the OnlineDriveTaskTracker of the active drive task and perform an updated routing to the final link L8. During the simulation, I track the predicted arrival time when doing the initial routing, after every diversion routing, and I track the final arrival time from the events.

Ideally, all cases should produce the same arrival time, only then the predictions are correct. Currently, the following happens:

Findings for the time being:

Now, to fix the issue and get correct predictions:

sebhoerl commented 2 years ago

Maybe even better would be an option VrpPaths.calcAndCreatePath(LinkTimePair diversionPoint, Link toLink, LeastCostPathCalculator router, TravelTime travelTime).

sebhoerl commented 2 years ago

The potential fixes are implemented in https://github.com/matsim-org/matsim-libs/commit/80a1e2471832e6c72c0a17486847a87cd036e5b0. However, theu invalidate some existing tests (but there 1 second was used arbitrarily it seems). It even "fixes" DefaultPassengerEngineTest where the predicted drop-off time needed to be corrected arbitrarily in the test. The -1 can now be removed and the test passes: https://github.com/matsim-org/matsim-libs/blob/d5e8fc8424aecf2438fe6009b686c9a0af7ea132/contribs/dvrp/src/test/java/org/matsim/contrib/dvrp/passenger/DefaultPassengerEngineTest.java#L95-L98

sebhoerl commented 2 years ago

And a third point in this series: Is VrpPaths.getLastLinkTT still an artifact from older versions of the class? Wouldn't it be more correct, to have

    public static double getLastLinkTT(Link lastLink, double time, TravelTime travelTime) {
        return travelTime.getLinkTravelTime(lastLink, time, null, null);
    }

Assuming that we are working with the QSimFreeFlowTravelTime here.

sebhoerl commented 2 years ago

And yet another issue :) I implemented another test, which goes from a link L0 along a chain of links to L8. And while I never divert, I calculate the diversion path to the next link L9. While traversing the links, most of the time we get the correct prediction of the theoretical arrival time at L9. However, once we are traversing L7, we get a wrong signal, because the diversion time from L8 is calculated wrongly. The problem here is that, currently, VrpPaths.createPath calculates the travel time of the last link as floor(length / speed), while for all other links it is this +1, because it is calculated that the vehicle needs to traverse the following node. This is not the case for the last link in the drive task, as the vehicle stops there. However, if we divert, we must take into account this missing second, because the diversion starts after passing the node. I will push the corresponding unit test and some fixes later on for that.

sebhoerl commented 2 years ago

Here is a little summary, some tests and potential fixes are to come.

Initial routing

       1         2         3
|**|--------*|--------*|--------|

1) For a diversion, we must not take into account FIRST_LINK as we have a smooth transition:

|**|--------*|--------*|--------|
             |--------*|--------|

2) Special case: Extending the route

                                   Added node here
|**|--------*|--------*|-------*|
                                |--------|

Summary

michalmac commented 2 years ago

Thanks for looking into this issue and providing a detailed analysis!

I am aware that the current approach is not exact, it's more a compromise between accuracy (+/-1 s ) and complexity. Of course, making it "more" exact is good, however there are limits to it. A few examples:

IMO VrpPaths and QSimFreeSpeedTravelTime are already unnecessarily complicated. Here is another example which is too complex (given the purpose) and easy to get wrong: https://github.com/matsim-org/matsim-libs/blob/c3143a8658a64a194d13462dd24565374b6e0a2d/contribs/drt/src/main/java/org/matsim/contrib/drt/optimizer/insertion/SingleInsertionDetourPathCalculator.java#L142-L145

Therefore, I am thinking about going in a different direction. Instead of fully following the QSim implementation (incl. potential bugs or arbitrary decisions), maybe we should assume (and even implement?) a super simple, deterministic, (time-dependent) no-congestion model (e.g. 0 s for moving over nodes, 0 s for departing, no waiting at the buffer etc.) where we can obtain 100% exact estimates. Such a "reference model" could be used for traffic-free DVRP simulations where we need a very precise assessment. It would be also useful for unit/integration tests of VRP algorithms. At the same time, in real-traffic simulations (run with QSim), those 1-second errors are usually negligibly small compared to all other traffic-related approximations, so we would simply ignore them.

BTW. Simulating activities, such as charging, picking up, dropping off has also a limited accuracy (+/- 1 time step).

michalmac commented 2 years ago

This is how the referenced code section could look like if we simplify our assumptions

Path path = router.calcLeastCostPath(fromLink.getToNode(), toLink.getFromNode(), departureTime, null, null); 
double lastLinkTT = VrpPaths.getLastLinkTT(toLink, departureTime + path.travelTime); 
michalmac commented 2 years ago

BTW. A quite recent finding while working on #1741 related to the idea of simplifying the TT model in DVRP:

I ran our "old" Berlin DRT scenario with 6000 DRT buses and 270 000 requests, free-flow traffic (link capacities increased x100, so basically no congestion), rejections turned off.

It turned out that having restrictiveBeelineSpeedFactor set to 0.5 (i.e. travel times 2x longer then the free-speed times) gave better results than restrictiveBeelineSpeedFactor set to 1.0 (although the latter was the "right" one for a free-float scenario). Interestingly, "0.5" shortened the planning horizon (due to TWs) and led to a better workload distribution across vehicles, higher sharing rates and lower waiting times.

This showed that the choice and precise tuning of the cost function may have a much higher impact on the obtained results than the use of precise travel times.

sebhoerl commented 2 years ago

Nice list of caveats :) Yes, I think a benchmark simulator could be a good way forward (but also see my remark further below).

However, what is your take on this in the short term? I would like to fix VrpPaths such that in well-defined conditions, we can predict QSim 100%:

This would allow me to set up some consistent test cases for porting the Alonso-Mora algorithm. Here, the timing is actually crucial if we don't want to have an extremely deteriorated service level under free flow conditions. Because the algorithm performs a re-matching of all vehicles and all request in every decision epoch, and the first assigned wait time is later on served as the wait time constraint for an agent (as if the service would communicate the expected arrival time to the customer and wants to stick to it). So we have the phenomenon that in one epoch a wait time of 05:00 is promised, and in the next epoch the exact same vehicle under free flow conditions is expected at 05:01 which will make the algorithm assign a new vehicle, and the whole logic cascades until a vehicle is close enough.

If I prepare a PR that covers the points above, do you see any obstacle for merging it?

michalmac commented 2 years ago

Sure, let's make a round with the 1-second issue fixes. However, in the long term we should think about some simplifications in this area. I think I agree with all your suggestions, but let's discuss them inside the PR.

BTW. Have you thought of making the solutions more stable, so the algorithm does not over-react to little changes in the ETA?

sebhoerl commented 2 years ago

Yes, this is what we were working on for TRB, basically making the algorithm more robust to fluctuations in travel time. This will also be ported.

michalmac commented 2 years ago

Awesome.