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

Custom cost matrices #555

Merged
merged 23 commits into from
Sep 30, 2021
Merged

Custom cost matrices #555

merged 23 commits into from
Sep 30, 2021

Conversation

jcoupey
Copy link
Collaborator

@jcoupey jcoupey commented Aug 20, 2021

Issue

This PR aims at pushing the separation between travel times and costs one step further by allowing to pass custom cost matrices in input. Fixes #415.

Tasks

  • Parse matrices.*.costs matrices in input
  • Adjust vroom::Input to hold the various types of matrices
  • Update vroom::CostWrapper to introduce various ways to compute/use costs depending on context
  • Define proper cost values when generating vroom::Route objects in solutions
  • Fix durations/costs separation leftovers
  • Update docs/API.md
  • Update CHANGELOG.md
  • review

@jcoupey jcoupey mentioned this pull request Sep 8, 2021
@jcoupey
Copy link
Collaborator Author

jcoupey commented Sep 8, 2021

Fixes #570

@jcoupey
Copy link
Collaborator Author

jcoupey commented Sep 10, 2021

While testing, I'm seeing situations where this PR currently hits:

assert(new_cost + best_gain == previous_cost);

This happens when custom costs and durations matrix are different. It means we have a faulty operator gain evaluation. I suspect this is a leftover from the costs/durations separation done a while back.

@jcoupey
Copy link
Collaborator Author

jcoupey commented Sep 10, 2021

We're getting there but I'm still seeing weird things going on. For example when using a cost matrix that is the duration matrix doubled, then for some instances the solving path is changed at some point and we get a different solution.

AFAICT this is related to a problem in UnassignedExchange gain evaluation. There are probably still a couple values involving durations instead of costs.

@jcoupey
Copy link
Collaborator Author

jcoupey commented Sep 13, 2021

After much digging, I think the last problems I'm seeing are not at all cost-related. My feeling is that comparing solutions across instances where we scale the costs is only a way to highlight a problem that is already present in the codebase, so I'll open a dedicated ticket for that.

@jcoupey jcoupey added this to the v1.11.0 milestone Sep 14, 2021
@jcoupey jcoupey added the API label Sep 14, 2021
@jcoupey jcoupey merged commit a791c54 into master Sep 30, 2021
@jcoupey jcoupey deleted the feature/custom-costs branch October 1, 2021 15:40
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.

Supply cost matrix and duration matrix separately
1 participant