Skip to content
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

Add max_distance constraint #991

Merged
merged 30 commits into from
Sep 25, 2023
Merged

Add max_distance constraint #991

merged 30 commits into from
Sep 25, 2023

Conversation

jcoupey
Copy link
Collaborator

@jcoupey jcoupey commented Sep 18, 2023

Issue

Fixes #354.

Tasks

  • Parse max_distance value in input
  • Store max_distance at vehicle level
  • Adjust Eval and CostWrapper to hold distances
  • Replicate max_travel_time logic in heuristics and local search to account for max distances constraints
  • Adjust vehicle ordering in heuristics to account for max_* constraints
  • Update docs/API.md
  • Update CHANGELOG.md
  • review

@jcoupey
Copy link
Collaborator Author

jcoupey commented Sep 18, 2023

The hard part about custom matrices in input is that it is not always clear what to do when durations are provided but not distances. It could be either that users expect to:

  1. get distances from the routing engine;
  2. not account for distances in the optimization as was the case previously.

Enforcing 1. would be a breaking change for "self-contained" instances (e.g. all usual benchmarks) with custom durations only that would not run any more without a routing engine. Enforcing 2. would keep the desired behavior for self-contained instances, but on the other hand it should be a valid use-case to only provide custom durations and expect distances to be fetched from the routing engine, then used in optimization.

So I went with deciding on 1. or 2. dynamically based on whether the -g flag is provided. If it is, it means that routing is expected anyway to fill in distances at route level, so we go with 1. If not, we assume that distances are not meaningful for optimization and we set a distances matrix filled with zeros.

@jcoupey
Copy link
Collaborator Author

jcoupey commented Sep 18, 2023

Follow-up note on custom matrices: we have no commitment so far to support custom distances so we can actually request that they're only provided if durations are also custom. This solves the dual version of the above question: providing only custom distances without custom durations is an input error.

@jcoupey jcoupey added this to the v1.14.0 milestone Sep 19, 2023
@jcoupey
Copy link
Collaborator Author

jcoupey commented Sep 21, 2023

One thing I overlooked is that adding a VIOLATION::MAX_DISTANCE in plan mode is not trivial because it requires to account for accumulated distances throughout the route, which we only do later on currently. Actually the scope of #957 is precisely to move that computation during route building now that we have distance matrices at hand.

I think the simplest way to move forward is to first merge support for max_distance in solving mode (this PR), then handle the MAX_DISTANCE violation in a separate ticket as a follow-up of #957.

@jcoupey
Copy link
Collaborator Author

jcoupey commented Sep 25, 2023

With this PR, distance range for vehicle is based on max_travel_time keys and values in the distance matrix. Now currently (until we have #957 implemented), the actual distances reported in output in routes and routes[].steps are computed based on the subsequent route requests to the routing engine. Since there is some rounding involved and the rounding is done on summed values instead of each distance pair, this can result in discrepancies.

For example with this PR as is, it is possible to get a route distance in output slightly over the max_travel_time value. Not a bug, just that the limitation is done on the distance matrix values, not on the distances from the route request showing up in output. This kind of mismatch should vanish with #957.

@jcoupey jcoupey merged commit 3f6f73c into master Sep 25, 2023
3 of 4 checks passed
@jcoupey jcoupey deleted the feature/max-distance branch September 25, 2023 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Maximum distance for vehicle
1 participant