Skip to content

Commit 25129a2

Browse files
committed
fix: correct UB and swapped bindings in set_cover pybind11 wrapper
Fix three bugs in ortools/set_cover/python/set_cover.cc: 1. VectorIntToVectorSubsetIndex: std::transform wrote to begin() of an empty vector (undefined behavior). Added reserve() + back_inserter(). 2. all_subsets property: same empty-vector UB pattern. Same fix. 3. GuidedTabuSearch: set_lagrangian_factor was bound to GetLagrangianFactor and vice versa. Added regression tests for all three code paths.
1 parent dde278d commit 25129a2

2 files changed

Lines changed: 36 additions & 4 deletions

File tree

ortools/set_cover/python/set_cover.cc

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,8 @@ using ::py::make_iterator;
7171
std::vector<SubsetIndex> VectorIntToVectorSubsetIndex(
7272
absl::Span<const BaseInt> ints) {
7373
std::vector<SubsetIndex> subs;
74-
std::transform(ints.begin(), ints.end(), subs.begin(),
74+
subs.reserve(ints.size());
75+
std::transform(ints.begin(), ints.end(), std::back_inserter(subs),
7576
[](int subset) -> SubsetIndex { return SubsetIndex(subset); });
7677
return subs;
7778
}
@@ -199,9 +200,11 @@ PYBIND11_MODULE(set_cover, m) {
199200
.def_property_readonly("all_subsets",
200201
[](SetCoverModel& model) -> std::vector<BaseInt> {
201202
std::vector<BaseInt> subsets;
203+
subsets.reserve(model.all_subsets().size());
202204
std::transform(
203205
model.all_subsets().begin(),
204-
model.all_subsets().end(), subsets.begin(),
206+
model.all_subsets().end(),
207+
std::back_inserter(subsets),
205208
[](const SubsetIndex element) -> BaseInt {
206209
return element.value();
207210
});
@@ -555,9 +558,9 @@ PYBIND11_MODULE(set_cover, m) {
555558
return heuristic.NextSolution(
556559
BoolVectorToSubsetBoolVector(in_focus));
557560
})
558-
.def("get_lagrangian_factor", &GuidedTabuSearch::SetLagrangianFactor,
561+
.def("set_lagrangian_factor", &GuidedTabuSearch::SetLagrangianFactor,
559562
arg("factor"))
560-
.def("set_lagrangian_factor", &GuidedTabuSearch::GetLagrangianFactor)
563+
.def("get_lagrangian_factor", &GuidedTabuSearch::GetLagrangianFactor)
561564
.def("set_epsilon", &GuidedTabuSearch::SetEpsilon, arg("r"))
562565
.def("get_epsilon", &GuidedTabuSearch::GetEpsilon)
563566
.def("set_penalty_factor", &GuidedTabuSearch::SetPenaltyFactor,

ortools/set_cover/python/set_cover_test.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,35 @@ def test_knights_cover_trivial(self):
222222
inv.check_consistency(set_cover.consistency_level.FREE_AND_UNCOVERED)
223223
)
224224

225+
def test_all_subsets_property(self):
226+
model = create_knights_cover_model(4, 4)
227+
all_subs = model.all_subsets
228+
self.assertLen(all_subs, model.num_subsets)
229+
self.assertEqual(all_subs, list(range(model.num_subsets)))
230+
231+
def test_focus_based_next_solution(self):
232+
model = create_knights_cover_model(8, 8)
233+
self.assertTrue(model.compute_feasibility())
234+
inv = set_cover.SetCoverInvariant(model)
235+
236+
focus = list(range(model.num_subsets))
237+
greedy = set_cover.GreedySolutionGenerator(inv)
238+
self.assertTrue(greedy.next_solution(focus))
239+
self.assertEqual(inv.num_uncovered_elements(), 0)
240+
241+
def test_guided_tabu_search_lagrangian_factor(self):
242+
model = create_knights_cover_model(8, 8)
243+
self.assertTrue(model.compute_feasibility())
244+
inv = set_cover.SetCoverInvariant(model)
245+
246+
greedy = set_cover.GreedySolutionGenerator(inv)
247+
self.assertTrue(greedy.next_solution())
248+
249+
gts = set_cover.GuidedTabuSearch(inv)
250+
gts.initialize()
251+
gts.set_lagrangian_factor(0.5)
252+
self.assertAlmostEqual(gts.get_lagrangian_factor(), 0.5)
253+
225254
# TODO(user): KnightsCoverGreedyAndTabu, KnightsCoverGreedyRandomClear,
226255
# KnightsCoverElementDegreeRandomClear, KnightsCoverRandomClearMip,
227256
# KnightsCoverMip

0 commit comments

Comments
 (0)