From dab44106d975ed804bd19576352244c585e83f9b Mon Sep 17 00:00:00 2001
From: jg-codes <53511569+jg-codes@users.noreply.github.com>
Date: Sat, 1 Aug 2026 20:37:43 +0200
Subject: [PATCH] fix: repair the set_cover all_subsets property

Reading model.all_subsets from Python terminates the interpreter. Two
separate defects in the same statement:

1. SetCoverModel::all_subsets() returns by value (set_cover_model.h:211),
   and the property called it twice, so begin() and end() came from two
   distinct temporaries. That iterator pair does not delimit a range, so
   the transform walks off the end of the first temporary's buffer. This
   is the dominant defect and it is the reason the property has never
   worked.

2. The output iterator was subsets.begin() on a default-constructed,
   zero-capacity vector, which is a write through an invalid iterator.

Both are fixed by binding the returned vector to a local and appending
through back_inserter(). Binding the local also removes a third full
copy of the vector that the reserve() call would otherwise make.

Note the neighbouring columns and rows properties are not affected:
columns() and rows() return const references (set_cover_model.h:189,
:192), so calling them twice is harmless. They pre-size and write
through begin(), which is correct for a reference-returning accessor.

VectorIntToVectorSubsetIndex had defect 2 as well and is fixed here for
consistency, but it is not reachable from Python: all of its callers take
absl::Span, and this module registers no absl::Span type caster. That is
reported separately.

Adds a regression test for the property. The focus-based paths cannot be
regression-tested from Python until the Span casters are addressed.

The swapped GuidedTabuSearch lagrangian getter/setter reported in the
first version of this PR was fixed upstream in a1e13b37, so that hunk is
dropped.
---
 ortools/set_cover/python/set_cover.cc      | 29 +++++++++++++---------
 ortools/set_cover/python/set_cover_test.py |  6 +++++
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/ortools/set_cover/python/set_cover.cc b/ortools/set_cover/python/set_cover.cc
index 82d0b4e2f4..46490a9ff2 100644
--- a/ortools/set_cover/python/set_cover.cc
+++ b/ortools/set_cover/python/set_cover.cc
@@ -72,7 +72,8 @@ using ::py::make_iterator;
 std::vector<SubsetIndex> VectorIntToVectorSubsetIndex(
     absl::Span<const BaseInt> ints) {
   std::vector<SubsetIndex> subs;
-  std::transform(ints.begin(), ints.end(), subs.begin(),
+  subs.reserve(ints.size());
+  std::transform(ints.begin(), ints.end(), std::back_inserter(subs),
                  [](int subset) -> SubsetIndex { return SubsetIndex(subset); });
   return subs;
 }
@@ -197,17 +198,21 @@ PYBIND11_MODULE(set_cover, m) {
              return make_iterator<>(IntIterator::begin(model.num_elements()),
                                     IntIterator::end(model.num_elements()));
            })
-      .def_property_readonly("all_subsets",
-                             [](SetCoverModel& model) -> std::vector<BaseInt> {
-                               std::vector<BaseInt> subsets;
-                               std::transform(
-                                   model.all_subsets().begin(),
-                                   model.all_subsets().end(), subsets.begin(),
-                                   [](const SubsetIndex element) -> BaseInt {
-                                     return element.value();
-                                   });
-                               return subsets;
-                             })
+      .def_property_readonly(
+          "all_subsets",
+          [](SetCoverModel& model) -> std::vector<BaseInt> {
+            // all_subsets() returns by value, so it must be
+            // bound to a local: calling it twice would give
+            // begin() and end() of two distinct temporaries.
+            const std::vector<SubsetIndex> all = model.all_subsets();
+            std::vector<BaseInt> subsets;
+            subsets.reserve(all.size());
+            std::transform(all.begin(), all.end(), std::back_inserter(subsets),
+                           [](const SubsetIndex element) -> BaseInt {
+                             return element.value();
+                           });
+            return subsets;
+          })
       .def("set_name", &SetCoverModel::SetName)
       .def("add_empty_subset", &SetCoverModel::AddEmptySubset, arg("cost"))
       .def(
diff --git a/ortools/set_cover/python/set_cover_test.py b/ortools/set_cover/python/set_cover_test.py
index 220e2cee51..5832dc430e 100644
--- a/ortools/set_cover/python/set_cover_test.py
+++ b/ortools/set_cover/python/set_cover_test.py
@@ -222,6 +222,12 @@ def test_knights_cover_trivial(self):
             inv.check_consistency(set_cover.consistency_level.FREE_AND_UNCOVERED)
         )
 
+    def test_all_subsets_property(self):
+        model = create_knights_cover_model(4, 4)
+        all_subsets = model.all_subsets
+        self.assertLen(all_subsets, model.num_subsets)
+        self.assertEqual(all_subsets, list(range(model.num_subsets)))
+
     # TODO(user): KnightsCoverGreedyAndTabu, KnightsCoverGreedyRandomClear,
     # KnightsCoverElementDegreeRandomClear, KnightsCoverRandomClearMip,
     # KnightsCoverMip