Draft: max detour constraint drt #2834
Conversation
to include max ride duration. The max ride duration is calculated in a similar manner as the max travel time. A new set of parameters (maxDetourAlpha, maxDetourBeta) are introduced in the drtCfg to calculate the max ride duration.
| } | ||
|
|
||
| final int pickupIdx = insertion.pickup.index; | ||
| Map<Id<Request>, Double> pickUps = new HashMap<>(ongoingRequests); |
There was a problem hiding this comment.
What is the point of copying this map?
There was a problem hiding this comment.
I think I understand the idea. However, I think copying the map and then adding even more stuff to it is a bit too heavy.
|
|
||
| public class MaxDetourInsertionCostCalculator implements InsertionCostCalculator, PassengerPickedUpEventHandler, PassengerDroppedOffEventHandler { | ||
|
|
||
| private final Map<Id<Request>, Double> ongoingRequests = new HashMap<>(); |
There was a problem hiding this comment.
This map can be read and written at the same time (events handling is executed in parallel to optimisation)
…matsim-libs into maxDetourConstraintDrt
controler -> controller
Fix the binding issue. But there are still some major problems in the insertion part.
|
Hi Nico, it seems that our hack does not work with the current insertion logic. The arrival time of the WayPoint is not implemented. I have discussed with Michal (@michalmac), and now we think it may be better to have some more fundamental implementation, rather than changing the current basic structure of the insertion. We can have a discussion after you return from your vacation. |
|
Hi Nico, How are you? Hope everything is running smoothly :) May I know if there is any update from your side on this topic? Shall we go ahead to change the structure of the insertion logic, or perhaps we can think of an alternative quick fix? Best regards, |
|
Just as comment, it might also be worth it thinking about adding this as a constraint here :) |
That's a great point! Thanks a lot, Sebastian! |
My suggestion would be to use the max detour time in pre-computation of slack times, i.e. before generating insertions. Otherwise, if we only post-process insertions, we may experience an "unexplainable" increase in computation time when changing from max travel time to max detour time. |
|
Hey @luchengqi7 ! sorry for my late reply, it was a bit busy. Maybe we can have a chat next week? I'll write a mail |
Sure, that sounds good! Thanks a lot! 👍 |
|
I just pushed a commit that reverts the changes in @nkuehnel @luchengqi7 I wanted to bring this up for your next week sync: static double getMaxTravelTime(DrtConfigGroup drtCfg, double unsharedRideTime) {
return Math.min(unsharedRideTime + drtCfg.maxAbsoluteDetour, drtCfg.maxTravelTimeAlpha * unsharedRideTime + drtCfg.maxTravelTimeBeta);
}
static double getMaxRideTime(DrtConfigGroup drtCfg, double unsharedRideTime) {
return Math.min(unsharedRideTime + drtCfg.maxAbsoluteDetour, drtCfg.maxDetourAlpha * unsharedRideTime + drtCfg.maxDetourBeta);
} |
We now make use the initial planned pick up time (+ a small buffer) + max ride duration to determine the latest arrival time. This will also be a fixed value, therefore, it will not impact the matching logic. The latest arrival time based on the original alpha beta value is also included here (with Math.min operation). The main change here is that we can no longer insert a lot of stops before the scheduled pick ups, as we limit the latest pick up time based on the initial scheduled pick up time. This should also be meaningful in the reality.
to the drtCfg
|
Hi @nkuehnel, today I have discussed with Michal, and he is happy about this new idea. Since there are some other changes and conflicts in this branch, shall I open a new PR in the current matsim-lib (upstream)? |
* Add DRT detour constraints Based on the PR #2834 * Consolidate two classes PromisedPickupTimeWindowDrtOfferAcceptor -> MaxDetourOfferAcceptor MaxDetourInsertionCostCalculator -> DefaultInsertionCostCalculator * Update SharingFactorTest.java * Update expected outputs * Unimportant commit To make sure the test will be based on the new expected values * temporarily disable check * Update some naming and description Based on Michal's suggestion. * Minor refactoring * Revert "temporarily disable check" This reverts commit d69429b. * taxi: update expected output plans in tests --------- Co-authored-by: Michal Maciejewski <[email protected]>, Nico Kühnel (nkuehnel)
|
The proposed changes in this PR have been merged to the master branch by PR #2997. |
No description provided.