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

VRP refactoring #10

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

informarte
Copy link
Contributor

The library has two models for the Capacitated Vehicle Routing Problem: A MIP model in vrp and a CP model in cvrp. Both directories contain instances but the instances in cvrp are a subset of those in vrp. The instance files have names like A-n32-k5.vrp.dzn where n is the number of customers and k is the number of vehicles. However, the instance files do not contain k and the models assume that k equals n.

This PR solves both issues:

  • It moves everything from vrp to cvrp.
  • It extends the instance files with k.
  • It changes the models to consider k.

Copy link
Member

@angee angee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good to me, there is just one issue: in the vrp.mzn file, K is assigned to N, so now by adding an assignment for K in the data files causes an error when running vrp.mzn:

MiniZinc: type error: multiple assignment to the same variable

In my opinion, the best and easiest fix for this would be to replace "int: K = N;" with "int: K;" in the vrp.mzn file.

@informarte
Copy link
Contributor Author

The first commit renames vrp/vrp.mzn to cvrp/cvrp_mip.mzn and the second commit replaces int K = N; with int: K;.

On my working copy:

> minizinc cvrp_cp.mzn simple2.dzn 
objective = 34;
vehicle = [2, 4, 1, 4, 2, 3, 4, 1, 2, 3, 4, 5, 6, 7, 1, 2, 3, 4, 5, 6, 7];
arrivalTime = [5, 10, 2, 9, 11, 1, 5, 0, 0, 0, 0, 0, 0, 0, 4, 15, 2, 13, 0, 0, 0];
successor = [5, 18, 15, 2, 16, 17, 4, 3, 1, 6, 7, 19, 20, 21, 9, 10, 11, 12, 13, 14, 8];
----------
==========

> minizinc cvrp_mip.mzn simple2.dzn 
x = [0, 0, 0, 1, 0, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0]
objective = 34
----------
==========

@angee
Copy link
Member

angee commented Oct 13, 2020

Of course, you are right, thanks for clarifying. Then everything should be OK 👍

@guidotack
Copy link
Member

Thanks a lot for your work on this! I think we'll need to reorganise the repositories a bit before merging this one. Currently this repo has a dual role: as a set of benchmarks, but also to collect previous challenge problems. For the challenge problems, I would like to avoid renaming and modifying models. It's probably best to set up a separate repository with all only the challenge models and instances, exactly as they were run. Then we're free to make changes to this one without causing too much confusion. Does that make sense?

@informarte
Copy link
Contributor Author

Yes, that makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants