-
Notifications
You must be signed in to change notification settings - Fork 470
feat(drt): explicit max ride constraints in drt #3922
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
base: main
Are you sure you want to change the base?
Conversation
…nt in promised time window offer acceptor
Hi Nico, have a look at this issue, I think this is very much related: #3757 I didn't check in detail yet what you did there, but making a difference between "vehicle arrival time" and "passenger dropoff time" and then "vehicle departure time" and "passenger pickup time" is indeed important and needs to be passed through from the InsertionGenerator all the way to the InsertionCostCalculator to handle variable stop durations properly. |
Regarding this PR, if I understand correctly, the point is that the slack does not take into account the ride duration (and max ride duration) if the existing requests yet? So we may find insertions that violate the max ride constraint of existing requests. Now this PR limits the slack such that it is reduced to the amount of time that is left in terms of ride time for the onboard requests without violating the constraints? So this is only a performance improvement to filter out early irrelevant insertions. Because right now these cases are later thrown out by the InsertionCostCalculator? Or this was not handled at all? I think the PR looks good, at the end not sure if this is fully related, but the pickup/dropoff fields will be quite useful for resolving #3757 I think. |
@sebhoerl correct it's about ensuring that existing requests' max ride duration is not exceeded. This was not possible so far in the cost calculation, it only worked if you limited the pickup delay as the pickup time window then became narrow enough to ensure that accepted requests do not violate max ride too heavily. I'll have a look at your linked issue |
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.
Thanks, Nico, for the nice update! It looks good, and I am looking forward to using the new constraint in my case studies!
One side comments: do you know how the computational speed is impacted? Hopefully there is no major impact :)
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.
Does this LATE_DIVERSION_VIOLATION_PENALTY also correspond to the latest arrival time?
A note based on a chat with Michal several years ago: these values are chosen as the initial values. And it may not be the ideal one, but it works for now.
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.
@luchengqi7
The latest arrival time still has its own penalty, these are all the penalties we have now:
MAX_WAIT_TIME_VIOLATION_PENALTY = 1;// 1 second of penalty per 1 second of late departure
MAX_TRAVEL_TIME_VIOLATION_PENALTY = 10;// 10 seconds of penalty per 1 second of late arrival
MAX_RIDE_TIME_VIOLATION_PENALTY = 10;// 10 seconds of penalty per 1 second of exceeded detour
LATE_DIVERSION_VIOLATION_PENALTY = 10;// 1 second of penalty per 1 second of late diversion of onboard requests
LATE_DIVERSION_VIOLATION_PENALTY = 10;// 10 second of penalty per 1 second of late diversion of onboard requests
The late diversion can be configured so that x seconds before the expected dropoff, passengers may not be detoured anymore, even if it would fit into the initial time window contract. However, this was introduced in an earlier PR and should not be affected by this one
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.
Ah and I havent had the chance to really test performance impacts. I can do that before merging
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.
I did some informal checking on performance and at least in one scenario it was progressing noticeably faster, but this was likely due to increased rejections. A more correct assessment of performance would be to verify it in an test case where rejections are exactly the same before/after this change..
Explicitly consider max ride duration constraints in the slack time calculation.
Previously, max ride duration was mainly accounted for in the insertion of new trips only, but not for existing trips. The workaround was to also set the maximum pickup delay, such that accepted rides pickups were not changing too much after insertion anymore.
To account for max ride durations explicitly, accepted requests now have additional fields for current (expected) pickup and dropoff times. These need to be updated, which is why I extracted the schedule timing updater interface that also updates these times for DRT...
@sebhoerl could you please also have a quick look, as I saw that sometimes for more complicated stop durations the max ride duration is exceeded by roughly the stop duration which might not be accounted for correctly in the updating of pickup and dropoff times (and, hence, slack times in the vehicle data entry).
Leaving this as a draft for now, feedback welcome