Skip to content

Commit fab05c8

Browse files
lperronlperron
committed
[CP-SAT] regroup hint code; fix more fuzzer bugs
1 parent 21a7a80 commit fab05c8

15 files changed

+533
-303
lines changed

‎ortools/sat/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -981,6 +981,7 @@ cc_library(
981981
"//ortools/port:proto_utils",
982982
"//ortools/util:logging",
983983
"//ortools/util:sorted_interval_list",
984+
"@com_google_absl//absl/base:log_severity",
984985
"@com_google_absl//absl/log:check",
985986
"@com_google_absl//absl/types:span",
986987
],

‎ortools/sat/cp_model_checker.cc‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,14 @@ std::string ValidateRoutesConstraint(const ConstraintProto& ct) {
636636
"All nodes in a route constraint must have incident arcs");
637637
}
638638

639+
// If the demands field is present, it must be of size nodes.size().
640+
if (!ct.routes().demands().empty() &&
641+
ct.routes().demands().size() != nodes.size()) {
642+
return absl::StrCat(
643+
"If the demands fields is set, it must be of size num_nodes:",
644+
nodes.size());
645+
}
646+
639647
return ValidateGraphInput(/*is_route=*/true, ct.routes());
640648
}
641649

@@ -793,6 +801,15 @@ std::string ValidateReservoirConstraint(const CpModelProto& model,
793801
}
794802
for (const LinearExpressionProto& expr : ct.reservoir().time_exprs()) {
795803
RETURN_IF_NOT_EMPTY(ValidateAffineExpression(model, expr));
804+
// We want to be able to safely put time_exprs[i]-time_exprs[j] in a linear.
805+
if (MinOfExpression(model, expr) <=
806+
-std::numeric_limits<int64_t>::max() / 4 ||
807+
MaxOfExpression(model, expr) >=
808+
std::numeric_limits<int64_t>::max() / 4) {
809+
return absl::StrCat(
810+
"Potential integer overflow on time_expr of a reservoir: ",
811+
ProtobufShortDebugString(ct));
812+
}
796813
}
797814
for (const LinearExpressionProto& expr : ct.reservoir().level_changes()) {
798815
RETURN_IF_NOT_EMPTY(ValidateConstantAffineExpression(model, expr));

‎ortools/sat/cp_model_expand.cc‎

Lines changed: 12 additions & 154 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ void ExpandReservoirUsingCircuit(int64_t sum_of_positive_demand,
5656
int64_t sum_of_negative_demand,
5757
ConstraintProto* reservoir_ct,
5858
PresolveContext* context) {
59-
SolutionCrush& crush = context->solution_crush();
6059
const ReservoirConstraintProto& reservoir = reservoir_ct->reservoir();
6160
const int num_events = reservoir.time_exprs_size();
6261

@@ -72,73 +71,6 @@ void ExpandReservoirUsingCircuit(int64_t sum_of_positive_demand,
7271
std::vector<int> level_vars(num_events);
7372
for (int i = 0; i < num_events; ++i) {
7473
level_vars[i] = context->NewIntVar(Domain(var_min, var_max));
75-
if (crush.HintIsLoaded()) {
76-
// The hint of active events is set later.
77-
crush.SetNewVariableHint(level_vars[i], 0);
78-
}
79-
}
80-
81-
// The hints of the active events, in the order they should appear in the
82-
// circuit. The hints are collected first, and sorted later.
83-
struct ReservoirEventHint {
84-
int index; // In the reservoir constraint.
85-
int64_t time;
86-
int64_t level_change;
87-
};
88-
std::vector<ReservoirEventHint> active_event_hints;
89-
bool has_complete_hint = false;
90-
if (crush.HintIsLoaded()) {
91-
has_complete_hint = true;
92-
for (int i = 0; i < num_events && has_complete_hint; ++i) {
93-
if (crush.VarHasSolutionHint(PositiveRef(reservoir.active_literals(i)))) {
94-
if (crush.LiteralSolutionHint(reservoir.active_literals(i))) {
95-
const std::optional<int64_t> time_hint =
96-
crush.GetExpressionSolutionHint(reservoir.time_exprs(i));
97-
const std::optional<int64_t> change_hint =
98-
crush.GetExpressionSolutionHint(reservoir.level_changes(i));
99-
if (time_hint.has_value() && change_hint.has_value()) {
100-
active_event_hints.push_back(
101-
{i, time_hint.value(), change_hint.value()});
102-
} else {
103-
has_complete_hint = false;
104-
}
105-
}
106-
} else {
107-
has_complete_hint = false;
108-
}
109-
}
110-
}
111-
// Update the `level_vars` hints by computing the level at each active event.
112-
if (has_complete_hint) {
113-
std::sort(active_event_hints.begin(), active_event_hints.end(),
114-
[](const ReservoirEventHint& a, const ReservoirEventHint& b) {
115-
return a.time < b.time;
116-
});
117-
int64_t current_level = 0;
118-
for (int i = 0; i < active_event_hints.size(); ++i) {
119-
int j = i;
120-
// Adjust the order of the events occurring at the same time, in the
121-
// circuit, so that, at each node, the level is between `var_min` and
122-
// `var_max`. For instance, if e1 = {t, +1} and e2 = {t, -1}, and if
123-
// `current_level` = 0, `var_min` = -1 and `var_max` = 0, then e2 must
124-
// occur before e1.
125-
while (j < active_event_hints.size() &&
126-
active_event_hints[j].time == active_event_hints[i].time &&
127-
(current_level + active_event_hints[j].level_change < var_min ||
128-
current_level + active_event_hints[j].level_change > var_max)) {
129-
++j;
130-
}
131-
if (j < active_event_hints.size() &&
132-
active_event_hints[j].time == active_event_hints[i].time) {
133-
if (i != j) std::swap(active_event_hints[i], active_event_hints[j]);
134-
current_level += active_event_hints[i].level_change;
135-
crush.UpdateVarSolutionHint(level_vars[active_event_hints[i].index],
136-
current_level);
137-
} else {
138-
has_complete_hint = false;
139-
break;
140-
}
141-
}
14274
}
14375

14476
// For the corner case where all events are absent, we need a potential
@@ -148,17 +80,8 @@ void ExpandReservoirUsingCircuit(int64_t sum_of_positive_demand,
14880
circuit->add_tails(num_events);
14981
circuit->add_heads(num_events);
15082
circuit->add_literals(all_inactive);
151-
if (has_complete_hint) {
152-
crush.SetNewVariableHint(all_inactive, active_event_hints.empty());
153-
}
15483
}
15584

156-
// The index of each event in `active_event_hints`, or -1 if the event's
157-
// "active" hint is false.
158-
std::vector<int> active_event_hint_index(num_events, -1);
159-
for (int i = 0; i < active_event_hints.size(); ++i) {
160-
active_event_hint_index[active_event_hints[i].index] = i;
161-
}
16285
for (int i = 0; i < num_events; ++i) {
16386
if (!reservoir.active_literals().empty()) {
16487
// Add self arc to represent absence.
@@ -175,11 +98,6 @@ void ExpandReservoirUsingCircuit(int64_t sum_of_positive_demand,
17598
circuit->add_tails(num_events);
17699
circuit->add_heads(i);
177100
circuit->add_literals(start_var);
178-
if (has_complete_hint) {
179-
crush.SetNewVariableHint(start_var,
180-
!active_event_hints.empty() &&
181-
active_event_hints.front().index == i);
182-
}
183101

184102
// Add enforced linear for demand.
185103
{
@@ -200,11 +118,6 @@ void ExpandReservoirUsingCircuit(int64_t sum_of_positive_demand,
200118
circuit->add_tails(i);
201119
circuit->add_heads(num_events);
202120
circuit->add_literals(end_var);
203-
if (has_complete_hint) {
204-
crush.SetNewVariableHint(end_var,
205-
!active_event_hints.empty() &&
206-
active_event_hints.back().index == i);
207-
}
208121
}
209122

210123
for (int j = 0; j < num_events; ++j) {
@@ -224,13 +137,6 @@ void ExpandReservoirUsingCircuit(int64_t sum_of_positive_demand,
224137
circuit->add_tails(i);
225138
circuit->add_heads(j);
226139
circuit->add_literals(arc_i_j);
227-
if (has_complete_hint) {
228-
const int hint_i_index = active_event_hint_index[i];
229-
const int hint_j_index = active_event_hint_index[j];
230-
crush.SetNewVariableHint(arc_i_j, hint_i_index != -1 &&
231-
hint_j_index != -1 &&
232-
hint_j_index == hint_i_index + 1);
233-
}
234140

235141
// Add enforced linear for time.
236142
{
@@ -261,6 +167,8 @@ void ExpandReservoirUsingCircuit(int64_t sum_of_positive_demand,
261167
}
262168
}
263169
}
170+
context->solution_crush().SetReservoirCircuitVars(reservoir, var_min, var_max,
171+
level_vars, *circuit);
264172

265173
reservoir_ct->Clear();
266174
context->UpdateRuleStats("reservoir: expanded using circuit.");
@@ -337,6 +245,9 @@ void ExpandReservoirUsingPrecedences(bool max_level_is_constraining,
337245

338246
// Canonicalize the newly created constraint.
339247
context->CanonicalizeLinearConstraint(new_cumul);
248+
249+
DCHECK(!PossibleIntegerOverflow(*context->working_model, new_linear->vars(),
250+
new_linear->coeffs()));
340251
}
341252

342253
reservoir_ct->Clear();
@@ -539,39 +450,6 @@ void ExpandIntMod(ConstraintProto* ct, PresolveContext* context) {
539450
return;
540451
}
541452

542-
bool has_complete_hint = false;
543-
bool enforced_hint = true;
544-
int64_t expr_hint = 0;
545-
int64_t mod_expr_hint = 0;
546-
int64_t target_expr_hint = 0;
547-
SolutionCrush& crush = context->solution_crush();
548-
if (crush.HintIsLoaded()) {
549-
has_complete_hint = true;
550-
for (const int lit : ct->enforcement_literal()) {
551-
if (!crush.VarHasSolutionHint(PositiveRef(lit))) {
552-
has_complete_hint = false;
553-
break;
554-
}
555-
enforced_hint = enforced_hint && crush.LiteralSolutionHint(lit);
556-
}
557-
if (has_complete_hint && enforced_hint) {
558-
has_complete_hint = false;
559-
std::optional<int64_t> hint = crush.GetExpressionSolutionHint(expr);
560-
if (hint.has_value()) {
561-
expr_hint = hint.value();
562-
hint = crush.GetExpressionSolutionHint(mod_expr);
563-
if (hint.has_value()) {
564-
mod_expr_hint = hint.value();
565-
hint = crush.GetExpressionSolutionHint(target_expr);
566-
if (hint.has_value()) {
567-
target_expr_hint = hint.value();
568-
has_complete_hint = true;
569-
}
570-
}
571-
}
572-
}
573-
}
574-
575453
// Create a new constraint with the same enforcement as ct.
576454
auto new_enforced_constraint = [&]() {
577455
ConstraintProto* new_ct = context->working_model->add_constraints();
@@ -583,13 +461,6 @@ void ExpandIntMod(ConstraintProto* ct, PresolveContext* context) {
583461
const int div_var = context->NewIntVar(
584462
context->DomainSuperSetOf(expr).PositiveDivisionBySuperset(
585463
context->DomainSuperSetOf(mod_expr)));
586-
if (has_complete_hint) {
587-
if (enforced_hint) {
588-
crush.SetNewVariableHint(div_var, expr_hint / mod_expr_hint);
589-
} else {
590-
crush.SetNewVariableHint(div_var, context->MinOf(div_var));
591-
}
592-
}
593464
LinearExpressionProto div_expr;
594465
div_expr.add_vars(div_var);
595466
div_expr.add_coeffs(1);
@@ -607,13 +478,6 @@ void ExpandIntMod(ConstraintProto* ct, PresolveContext* context) {
607478
.IntersectionWith(context->DomainSuperSetOf(expr).AdditionWith(
608479
context->DomainSuperSetOf(target_expr).Negation()));
609480
const int prod_var = context->NewIntVar(prod_domain);
610-
if (has_complete_hint) {
611-
if (enforced_hint) {
612-
crush.SetNewVariableHint(prod_var, expr_hint - target_expr_hint);
613-
} else {
614-
crush.SetNewVariableHint(prod_var, context->MinOf(prod_var));
615-
}
616-
}
617481
LinearExpressionProto prod_expr;
618482
prod_expr.add_vars(prod_var);
619483
prod_expr.add_coeffs(1);
@@ -633,41 +497,34 @@ void ExpandIntMod(ConstraintProto* ct, PresolveContext* context) {
633497
AddLinearExpressionToLinearConstraint(prod_expr, -1, lin);
634498
AddLinearExpressionToLinearConstraint(target_expr, -1, lin);
635499

500+
context->solution_crush().SetIntModExpandedVars(*ct, div_var, prod_var,
501+
context->MinOf(div_var),
502+
context->MinOf(prod_var));
636503
ct->Clear();
637504
context->UpdateRuleStats("int_mod: expanded");
638505
}
639506

640507
void ExpandNonBinaryIntProd(ConstraintProto* ct, PresolveContext* context) {
641508
CHECK_GT(ct->int_prod().exprs_size(), 2);
642-
SolutionCrush& crush = context->solution_crush();
643509
std::deque<LinearExpressionProto> terms(
644510
{ct->int_prod().exprs().begin(), ct->int_prod().exprs().end()});
511+
std::vector<int> new_vars;
645512
while (terms.size() > 2) {
646513
const LinearExpressionProto& left = terms[0];
647514
const LinearExpressionProto& right = terms[1];
648515
const Domain new_domain =
649516
context->DomainSuperSetOf(left).ContinuousMultiplicationBy(
650517
context->DomainSuperSetOf(right));
651518
const int new_var = context->NewIntVar(new_domain);
652-
if (crush.HintIsLoaded()) {
653-
const std::optional<int64_t> left_hint =
654-
crush.GetExpressionSolutionHint(left);
655-
const std::optional<int64_t> right_hint =
656-
crush.GetExpressionSolutionHint(right);
657-
if (left_hint.has_value() && right_hint.has_value()) {
658-
crush.SetNewVariableHint(new_var,
659-
left_hint.value() * right_hint.value());
660-
}
661-
}
519+
new_vars.push_back(new_var);
662520
LinearArgumentProto* const int_prod =
663521
context->working_model->add_constraints()->mutable_int_prod();
664522
*int_prod->add_exprs() = left;
665523
*int_prod->add_exprs() = right;
666524
int_prod->mutable_target()->add_vars(new_var);
667525
int_prod->mutable_target()->add_coeffs(1);
668526
terms.pop_front();
669-
terms.pop_front();
670-
terms.push_back(int_prod->target());
527+
terms.front() = int_prod->target();
671528
}
672529

673530
LinearArgumentProto* const final_int_prod =
@@ -676,6 +533,7 @@ void ExpandNonBinaryIntProd(ConstraintProto* ct, PresolveContext* context) {
676533
*final_int_prod->add_exprs() = terms[1];
677534
*final_int_prod->mutable_target() = ct->int_prod().target();
678535

536+
context->solution_crush().SetIntProdExpandedVars(ct->int_prod(), new_vars);
679537
context->UpdateRuleStats(absl::StrCat(
680538
"int_prod: expanded int_prod with arity ", ct->int_prod().exprs_size()));
681539
ct->Clear();

‎ortools/sat/cp_model_expand_test.cc‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -701,7 +701,8 @@ TEST(IntProdExpandTest, TestLargerAffineProd) {
701701

702702
TEST(IntProdExpansionTest, ExpandNonBinaryIntProdPreservesSolutionHint) {
703703
CpModelProto initial_model = ParseTestProto(R"pb(
704-
variables { domain: [ 0, 100 ] }
704+
variables { domain: [ 0, 500 ] }
705+
variables { domain: [ 0, 10 ] }
705706
variables { domain: [ 0, 10 ] }
706707
variables { domain: [ 0, 10 ] }
707708
variables { domain: [ 0, 10 ] }
@@ -711,15 +712,16 @@ TEST(IntProdExpansionTest, ExpandNonBinaryIntProdPreservesSolutionHint) {
711712
exprs { vars: 1 coeffs: 1 }
712713
exprs { vars: 2 coeffs: 1 }
713714
exprs { vars: 3 coeffs: 1 }
715+
exprs { vars: 4 coeffs: 1 }
714716
}
715717
}
716718
objective {
717719
vars: [ 0 ]
718720
coeffs: [ 1 ]
719721
}
720722
solution_hint {
721-
vars: [ 0, 1, 2, 3 ]
722-
values: [ 60, 3, 4, 5 ]
723+
vars: [ 0, 1, 2, 3, 4 ]
724+
values: [ 360, 3, 4, 5, 6 ]
723725
}
724726
)pb");
725727

0 commit comments

Comments
 (0)