Speed up Expr __add__ and __iadd__

[Zeroto521]

This pr is 1.75x faster than the master branch.

• Optimized before: 9.0072 seconds
• Optimized after: 5.1427 seconds

from timeit import timeit

from pyscipopt import Model


m = Model()

n = 1_000
x = m.addMatrixVar(n)
e1 = x.sum()

y = m.addMatrixVar(n)
e2 = y.sum()

number = 100_000
cost = timeit(lambda: e1 + e2, number=number)
print(f"Cost of adding two expressions over {number:,} runs: {cost:.4f} seconds")

[Copilot]

Pull request overview

This PR optimizes Expr.__add__ / Expr.__iadd__ by moving term merging into a C-level helper (_to_dict) that iterates dictionaries via PyDict_Next, aiming to reduce Python-level overhead in expression addition.

Changes:

• Reworked Expr.__add__ and Expr.__iadd__ to use a new C-level merge helper for Expr + Expr / Expr += Expr.
• Added unit tests covering Expr + Expr and Expr += Expr basic behavior.
• Documented the optimization in the changelog.

Reviewed changes

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

File	Description
src/pyscipopt/expr.pxi	Implements faster term merging for addition/in-place addition via _to_dict and updates the dunder methods to use it.
tests/test_expr.py	Adds tests validating string representations after Expr + Expr and Expr += Expr.
CHANGELOG.md	Notes the new optimization in the Unreleased section.

---

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

[codecov]

Codecov Report

❌ Patch coverage is 95.65217% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 57.50%. Comparing base (75ccba9) to head (80aa0f1).
⚠️ Report is 13 commits behind head on master.

Files with missing lines	Patch %	Lines
src/pyscipopt/expr.pxi	95.65%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1205      +/-   ##
==========================================
+ Coverage   57.17%   57.50%   +0.33%     
==========================================
  Files          26       26              
  Lines        5674     5728      +54     
==========================================
+ Hits         3244     3294      +50     
- Misses       2430     2434       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Joao-Dionisio]

Hey @Zeroto521 , I'll take a look soon. In the meantime, how would you feel about taking a look at qpsolvers/qpsolvers#347? It's about adding SCIP support to an unified QP-solver. I thought about you because you're both into numpy and the matrix API, so it might be a good fit.

By the way, absolutely 0.0000000% pressure to do anything at all, of course!!

[Zeroto521]

Hey @Zeroto521 , I'll take a look soon. In the meantime, how would you feel about taking a look at qpsolvers/qpsolvers#347? It's about adding SCIP support to an unified QP-solver. I thought about you because you're both into numpy and the matrix API, so it might be a good fit.

By the way, absolutely 0.0000000% pressure to do anything at all, of course!!

Okay, I'll check that out later.

[Joao-Dionisio]

Hey @Zeroto521 ! Just a couple things. With

if (other_v := <double>(<object>v_ptr)) == 0:
      continue

you are silently dropping zero-coefficient terms from other, right? The old code did terms[v] = terms.get(v, 0.0) + c unconditionally, so if other.terms had a key v with coefficient 0.0 that wasn't already in self.terms, the resulting dict would still contain v: 0.0. With this PR it doesn't. Was this taken into account?

And the other thing is about the changelog, can you move it to the unreleased section, please?

[Zeroto521]

Hey @Zeroto521 ! Just a couple things. With

if (other_v := <double>(<object>v_ptr)) == 0:
      continue

you are silently dropping zero-coefficient terms from other, right? The old code did terms[v] = terms.get(v, 0.0) + c unconditionally, so if other.terms had a key v with coefficient 0.0 that wasn't already in self.terms, the resulting dict would still contain v: 0.0. With this PR it doesn't. Was this taken into account?

And the other thing is about the changelog, can you move it to the unreleased section, please?

Done

[Copilot]

Pull request overview

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

[Joao-Dionisio]

Thank you @Zeroto521 once again!