Skip to content

[ENH] set_cover Python: add SetCoverLagrangian bindings - #5122

Open
jg-codes wants to merge 1 commit into
google:mainfrom
jg-codes:add-set-cover-lagrangian-python-bindings
Open

[ENH] set_cover Python: add SetCoverLagrangian bindings#5122
jg-codes wants to merge 1 commit into
google:mainfrom
jg-codes:add-set-cover-lagrangian-python-bindings

Conversation

@jg-codes

@jg-codes jg-codes commented Apr 3, 2026

Copy link
Copy Markdown

Disclosure: This PR was drafted with AI assistance (Claude). I reviewed every line and built and ran it myself; see the verification section for exactly what that covered.

What

Adds Python bindings for the existing SetCoverLagrangian C++ class, resolving the TODO(user) at the end of set_cover.cc. No algorithmic changes; this only exposes what already exists in C++.

Why

The Python set_cover module exposes the greedy, steepest, GLS, tabu and dual-ascent optimizers, but not the Lagrangian relaxation code. SetCoverLagrangian::ComputeLowerBound() has no Python entry point at all, so Python users who want a Lagrangian dual bound, or multiplier-guided solutions built on top of one, have to reimplement the subgradient loop in Python and give up the compiled implementation.

DualAscentOptimizer already exposes a dual bound from Python, so this binding is not the only route to one. It is a genuinely different relaxation, and the question worth answering before adding 140 lines for it is whether that difference matters in practice. I benchmarked both, freshly, on this build.

Benchmark: does the new bound add anything DualAscentOptimizer does not?

Four OR-Library SCP instances (Beasley's OR-Library, scp41, scp42, scpa1, scpb1; a small sample given the time available, disclosed rather than silently limited), read via the already-bound read_orlib_scp. For each: a primal solution from GreedySolutionOptimizer + SteepestSearch, then two lower bounds computed independently against that same primal cost.

instance subsets elements primal dual ascent LB dual gap Lagrangian LB Lagrangian gap dual time Lagrangian time
scp41 1000 200 438 383-384 12.3-12.6% 420.96 3.89% ~3 ms ~0.31 s
scp42 1000 200 547 447-467 14.6-18.3% 511.10 6.56% ~3 ms ~0.31 s
scpa1 3000 300 271 206-210 22.5-24.0% 243.94 9.98% ~10 ms ~0.49 s
scpb1 3000 300 73 48-49 32.9-34.3% 62.65 14.18% ~20 ms ~0.63 s

Dual ascent ranges are two independent runs with set_num_random_passes(50), 10x the pass count needed to plateau (5 passes already gave the same bound within noise; 50 was to rule out an under-configured baseline before trusting the comparison). The Lagrangian bound was identical across both runs; ComputeLowerBound has no randomization.

Across these four instances, the Lagrangian bound cuts the gap to the primal solution by roughly 2.5 to 3 times versus dual ascent, consistently. It also costs roughly 15 to 60 times more wall clock time, because ComputeLowerBound runs a fixed 1000-iteration subgradient loop (set_cover_lagrangian.cc), while dual ascent's passes are individually far cheaper. That is a real trade-off, not a free improvement: this binding is for offline or batch bound computation, not an interactive path. ComputeLowerBound also returns the reduced costs and multipliers alongside the bound, which DualAscentOptimizer does not expose; those are what a caller would need to build a reduced-cost-guided heuristic on top of the bound.

Two things the build surfaced that are worth flagging on their own

Building this confirmed two defects in SetCoverLagrangian that a Python (or C++) caller would hit regardless of this PR:

  1. ThreePhase() is declared but never defined. set_cover_lagrangian.h:135 declares void ThreePhase(Cost upper_bound);; there is no definition anywhere in set_cover_lagrangian.cc or elsewhere in the tree. Binding it produces undefined symbol: _ZN19operations_research18SetCoverLagrangian10ThreePhaseEd at module import, which fails the whole extension, not just that one call. I have left it unbound, with a comment explaining why.
  2. ComputeLowerBound() null-dereferences unless UseNumThreads() is called first. thread_pool_ is nullptr until UseNumThreads() runs (set_cover_lagrangian.h:60,64), but ComputeLowerBound() unconditionally calls the Parallel* methods, which do thread_pool_->Schedule(...). On a freshly constructed SetCoverLagrangian, compute_lower_bound() segfaults. My test calls use_num_threads() first and documents why; nothing in the header states this precondition.

Neither is something this PR can fix without changing set_cover_lagrangian.cc/.h, which is out of scope for a Python-bindings PR. Flagging both here since I found them building this.

Methods exposed

Python method C++ method
initialize_lagrange_multipliers() InitializeLagrangeMultipliers
compute_reduced_costs(costs, multipliers) ComputeReducedCosts
parallel_compute_reduced_costs(costs, multipliers) ParallelComputeReducedCosts
compute_subgradient(reduced_costs) ComputeSubgradient
parallel_compute_subgradient(reduced_costs) ParallelComputeSubgradient
compute_lagrangian_value(reduced_costs, multipliers) ComputeLagrangianValue
parallel_compute_lagrangian_value(reduced_costs, multipliers) ParallelComputeLagrangianValue
update_multipliers(step_size, lagrangian_value, upper_bound, reduced_costs, multipliers) UpdateMultipliers
parallel_update_multipliers(...) ParallelUpdateMultipliers
compute_gap(reduced_costs, solution, multipliers) ComputeGap
compute_lower_bound(costs, upper_bound) ComputeLowerBound
use_num_threads(n) UseNumThreads

Optimize() and ThreePhase() are not exposed; see above for both.

These are the 12 remaining public methods SetCoverLagrangian declares itself. The inherited SetCoverOptimizer surface (time limits, run_time(), ResetLimits()) is not exposed here, and is not exposed for any other class in this file either.

StrongVector types (SubsetCostVector, ElementCostVector) are converted to and from list[float], consistent with the existing subset_costs property. UpdateMultipliers returns the updated multipliers, since the C++ out-pointer is not idiomatic in Python.

List parameters are typed const std::vector<double>&, not absl::Span<const double>. This module registers no absl::Span type caster, so a Span-typed parameter cannot be satisfied from Python at all: pybind11 renders it as the raw C++ type in the signature and rejects any list. The existing Span-typed bindings in this file have the same problem; reported separately.

Changes

  • ortools/set_cover/python/set_cover.cc: include, using declarations for Cost / ElementCostVector / SetCoverLagrangian, a VectorDoubleToElementCostVector helper mirroring the existing VectorDoubleToSubsetCostVector, and the py::class_<SetCoverLagrangian> block.
  • ortools/set_cover/python/set_cover_test.py: 6 tests: multiplier initialization, reduced costs, subgradient and Lagrangian value, multiplier update, lower bound, and serial/parallel agreement.
  • ortools/set_cover/python/BUILD.bazel: added the //ortools/set_cover:set_cover_lagrangian dep.
  • ortools/set_cover/python/CMakeLists.txt: no change needed, the pybind target already links ::ortools.

Verification

Built and tested on Linux (Bazel 8.7.0, x86_64) against main at d9c0910:

bazel test //ortools/set_cover/python:set_cover_test   PASSED

clang-format clean against the repo .clang-format; the Python file is black clean at the file's existing line width. The benchmark above ran against this same build.

Notes

Checklist

  • Read CONTRIBUTING.md and the PR template, targeting main
  • CLA signed
  • Minimal diff: bindings, tests, and one build dep
  • No unrelated formatting or refactoring changes
  • AI-assisted: disclosed above

@jg-codes
jg-codes force-pushed the add-set-cover-lagrangian-python-bindings branch from 196eaa5 to fad511d Compare August 1, 2026 22:36
Exposes the existing C++ SetCoverLagrangian class to Python via
pybind11, resolving the TODO(user) at the end of set_cover.cc.

Binds 12 of the class's public methods: multiplier initialization,
reduced costs, subgradient, Lagrangian value, multiplier updates, gap
computation, lower bound computation, and the parallel variants.

Two methods are deliberately left unbound:

- Optimize(): the header documents it as a dummy implementation and
  says the class is meant to be used through ComputeLowerBound().
- ThreePhase(): declared in set_cover_lagrangian.h:135, defined
  nowhere in the tree. Binding it links but produces an undefined
  symbol at module import, which fails the whole extension. Left
  unbound with a comment recording why.

ComputeLowerBound() also turned out to null-dereference on a freshly
constructed SetCoverLagrangian: it unconditionally calls the Parallel*
methods, and those require the thread pool that UseNumThreads()
constructs, which is null until that method is called. The test calls
use_num_threads() first and documents the precondition, since nothing
in the header states it.

The list-valued parameters are typed const std::vector<double>&
rather than absl::Span<const double>. No absl::Span type caster is
registered in this module, so a Span-typed parameter cannot be
satisfied from Python at all: pybind11 renders it as the raw C++ type
in the signature and rejects any list. The existing Span-typed
bindings in this file are unusable from Python for the same reason,
reported separately.

StrongVector types are converted to and from list[float], consistent
with the existing subset_costs property. UpdateMultipliers() mutates
through a pointer in C++; the Python binding returns the updated
multipliers instead.

Adds the //ortools/set_cover:set_cover_lagrangian bazel dep and 6
tests. CMake needs no change: the pybind target already links
::ortools.

Built and tested on Linux (Bazel 8.7.0) against main at d9c0910:
bazel test //ortools/set_cover/python:set_cover_test passes.
@jg-codes
jg-codes force-pushed the add-set-cover-lagrangian-python-bindings branch from fad511d to 4dad6ea Compare August 1, 2026 22:38
@jg-codes

jg-codes commented Aug 1, 2026

Copy link
Copy Markdown
Author

@Mizux, rebased onto current main and force-pushed. This no longer depends on #5121.

What changed since the version you would have seen in April:

  • The bug fixes that were bundled in here are gone. The GuidedTabuSearch lagrangian getter/setter swap was fixed by your a1e13b37; the remaining vector-transform defect now lives only in [BUG] set_cover Python: fix UB in the all_subsets property #5121, branch jg-codes:fix-set-cover-python-bindings. This PR is purely additive and reviewable on its own.
  • Dropped a stray BUILD.bazel edit that removed package(default_visibility = ...). That was unintended. The only build change now is the //ortools/set_cover:set_cover_lagrangian dep.
  • Ported the tests to the post-export API (GreedySolutionOptimizer, optimize()).
  • The list parameters are typed const std::vector<double>&, not absl::Span<const double>. I wrote them as spans first, to match the surrounding code, then found that this module registers no absl::Span caster at all: pybind11 renders such a parameter as the raw C++ type and rejects every list, so inv.compute_coverage_in_focus([0, 1, 2]) raises TypeError on the shipped wheel. Span-typed parameters here are unreachable from Python. The existing ones in this file have the same problem; filed separately.
  • Optimize() is not exposed. set_cover_lagrangian.h calls OptimizeImpl() "a dummy implementation ... that is not used", and the class comment says the class "is intended to be used only for ComputeLowerBound(). FOR THE TIME BEING".

Two things the build turned up that were not visible from reading the header alone:

  • ThreePhase() is declared but never defined. set_cover_lagrangian.h:135 declares it; there is no implementation anywhere in the tree. Binding it produced undefined symbol: ...ThreePhase... at module import and failed the whole extension. Left it unbound with a comment.
  • ComputeLowerBound() segfaults unless UseNumThreads() is called first. The thread_pool_ member is null until UseNumThreads() runs, but ComputeLowerBound() unconditionally calls the parallel variants, which dereference it. My test calls use_num_threads() before compute_lower_bound(); nothing in the header documents that this is required.

I built and ran a small benchmark on this exact build to check whether the new bound adds anything DualAscentOptimizer does not, since that class is already bound and also produces a lower bound. Numbers and methodology are in the PR description. Short version: on four OR-Library instances, the Lagrangian bound cut the gap to a primal solution by roughly 2.5 to 3 times versus dual ascent, at roughly 15 to 60 times the wall-clock cost. A real trade-off, not a free improvement, and worth knowing before deciding whether this earns its place.

Two questions where I would rather follow your preference than guess:

  1. Scope. This exposes the 12 remaining methods SetCoverLagrangian declares itself (Optimize() and ThreePhase() excluded, for the reasons above), not the inherited SetCoverOptimizer surface, none of which is bound for any class in this file. If you would rather keep the Python API to compute_lower_bound alone while the class is still marked "FOR THE TIME BEING", I will trim it; the benchmark above is my case for keeping the rest.
  2. update_multipliers returns the updated multipliers instead of mutating through a pointer, since an out-parameter is not idiomatic in Python. Happy to mirror the C++ signature instead if you prefer.

CMakeLists.txt needs no change: the pybind target already links ::ortools.

@jg-codes

jg-codes commented Aug 1, 2026

Copy link
Copy Markdown
Author

Filed the Span/focus-API defect as its own issue: #5278.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant