improve chgReoptObjective performance

[a-schmitt]

For medium sized production problems using reoptimization chgReoptObjective leads to quite some bottleneck.

The reason is some O (m * n) (m=#scip variables, n=#variables with non null objective) loop.
The current code seems to assume that the scip c function needs all m scip variables as input.
However, only the n variables to change are needed (see https://github.com/scipopt/scip/blob/775ad66cb8edde846e16d913009b30add5c6d07b/src/scip/scip_prob.c#L1294), reducing the complexity to O ( n ).

Changes:

• Improve the loop
• Correct the doc string
• Add some tests

[Joao-Dionisio]

Thank you, @a-schmitt ! This does look very useful! Can you please run the before and after on a bigger problem, to archive the performance improvement?

Something I'm wondering is about the removal of the _varArray() wrapper, it seems we are losing some type safety and memory management.

[Copilot]

Pull request overview

This PR optimizes Model.chgReoptObjective to avoid O(m·n) scans over all SCIP variables by passing only the variables that appear in the new objective, and adds tests and documentation to cover the updated behavior.

Changes:

• Reimplemented Model.chgReoptObjective to build dense arrays of only the variables present in the Expr and call SCIPchgReoptObjective with those, including a special-case path for zero-objective expressions.
• Updated the chgReoptObjective docstring to describe the reoptimization behavior and the Expr-based API accurately.
• Extended tests/test_reopt.py with additional cases for maximize sense, sparse objectives with many variables, and zero objective, and documented this improvement in CHANGELOG.md.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
src/pyscipopt/scip.pxi	Refactored Model.chgReoptObjective to compute nvars from the Expr terms, populate _vars/_coeffs arrays directly from the terms (excluding CONST), handle the zero-variable case via a NULL call, and adjusted the docstring accordingly.
tests/test_reopt.py	Added tests covering maximize-sense reoptimization, many-variable sparse objective reoptimization, and the zero-objective case (including feasibility and objective value checks), plus a clarifying docstring for the original test.
CHANGELOG.md	Recorded the improved chgReoptObjective() performance under unreleased changes.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[a-schmitt]

Thank you, @a-schmitt ! This does look very useful! Can you please run the before and after on a bigger problem, to archive the performance improvement?

Thanks for your effort!

I unscientifically tested on a toy problem

def benchmark(n, n_obj=10, n_iters=5):
    m = Model()
    m.enableReoptimization()
    x = [m.addVar() for i in range(n)]

    times = []
    for i in range(n_iters):
        m.freeReoptSolve()
        obj = quicksum(x[j] for j in range(i, n))

        t0 = time.perf_counter()
        m.chgReoptObjective(obj)
        t1 = time.perf_counter()

        times.append((t1 - t0) * 1000)

    return times

the proposed version

       n    mean_ms   stdev_ms
------------------------------
    1000       0.132      0.016
    5000       0.789      0.165
   10000       1.553      0.092

the old version

       n    mean_ms   stdev_ms
------------------------------
    1000     134.913      0.976
    5000    3432.795     63.303
   10000   14155.320    173.048

so something like ~14s vs ~2ms per objective change for the 10k variable problem and the difference scales with the problem size.

Something I'm wondering is about the removal of the _varArray() wrapper, it seems we are losing some type safety and memory management.

I don't like that the _varArray() wrapper approach uses a malloc for each individual variable.
Collecting the vars in a python list then giving the list to _varArray() is a little bit slower compared to the current suggestion. I added an explicit type check. However, I'm also not super against using the _varArray(). Whatever you prefer.

[Joao-Dionisio]

Collecting the vars in a python list then giving the list to _varArray() is a little bit slower compared to the current suggestion. I added an explicit type check.

True, but the major speedup comes from the better complexity. It just itches a bit not to have a centralized method for memory handling. Plus, it's safer if SCIPchgReoptObjective() ever fails.

Can you run another benchmark with this change? If it's significant enough, we can discard it.

[a-schmitt]

I added the wrapper within the loop similar how it is done in _createConsLinear. The timeloss is not significant.

I also removed the assert len(term) == 1, since there is already a check for the degree of the expression.

[Joao-Dionisio]

Thank you, @a-schmitt , and very cool that your first Github contribution was so mature!