Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: google/or-tools
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: main
Choose a base ref
...
head repository: jg-codes/or-tools
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: fix-set-cover-python-bindings
Choose a head ref
Checking mergeability… Don’t worry, you can still create the pull request.
  • 1 commit
  • 2 files changed
  • 1 contributor

Commits on Aug 1, 2026

  1. 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 a1e13b3, so that hunk is
dropped.
    @jg-codes
    jg-codes committed Aug 1, 2026
    Configuration menu
    Copy the full SHA
    dab4410 View commit details
    Browse the repository at this point in the history
Loading