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 with breaks minor bug? #4234

Open
vuongdanghuy opened this issue May 14, 2024 · 4 comments
Open

VRP with breaks minor bug? #4234

vuongdanghuy opened this issue May 14, 2024 · 4 comments
Assignees
Labels
Bug Solver: Routing - break Routing break related issue Solver: Routing Uses the Routing library and the original CP solver
Milestone

Comments

@vuongdanghuy
Copy link

What version of OR-Tools and what language are you using?
Version: stable branch
Language: C++

Which solver are you using (e.g. CP-SAT, Routing Solver, GLOP, BOP, Gurobi)
Routing Library

What operating system (Linux, Windows, ...) and version?
Linux Mint 21.3

What did you do?
Using VRP with breaks. GlobalVehicleBreaksConstraint will be added to the Solver.

What did you expect to see
I believe here we're updating StartMin value of the arc path_[pos] -> path_[pos + 1] to EndMin value of the break interval. This arc StartMin is CumulVar(path_[pos])->Min() - arc_start_offset. So instead of SetMin(CapSub(interval_end_min, arc_start_offset)) it should be SetMin(CapAdd(interval_end_min, arc_start_offset)).

// Interval not after.
if (interval_start_max < arc_end_min) {
interval->SetEndMax(arc_start_max);
if (interval_is_performed) {
dimension_->CumulVar(path_[pos])
->SetMin(CapSub(interval_end_min, arc_start_offset));
}
}
continue;

@Mizux Mizux added Bug Solver: Routing Uses the Routing library and the original CP solver Solver: Routing - break Routing break related issue labels May 23, 2024
@Mizux Mizux added this to the v10.0 milestone May 23, 2024
@ghost
Copy link

ghost commented May 23, 2024

You are correct, this should be a CapAdd() instead of a CapSub().
The current SetMin() is weaker than it could be.
Thank you for the report!

@varshneydevansh
Copy link

So all we have to do is to replace the CapSub with the CapAdd ?

Or is there anything else which I should consider?

@vuongdanghuy
Copy link
Author

yeah, replace CapSub by CapAdd will do

@Mizux Mizux assigned Mizux and ghost Jun 5, 2024
@varshneydevansh
Copy link

Okay creating a PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Solver: Routing - break Routing break related issue Solver: Routing Uses the Routing library and the original CP solver
Projects
None yet
Development

No branches or pull requests

4 participants