Skip to content

Conversation

@daljit46
Copy link
Member

Fixes #3260.
The following change was introduced in #3182:

-    std::fill(active.begin(), active.end(), false);
-    if (num_eq > 0)
-      std::fill(active.begin() + num_ineq, active.end(), true);
+    active.setZero();
+    active.tail(num_ineq).setOnes();

We were mistakenly setting the last num_ineq entries active, while we should be doing the opposite: set the last num_eq entries active. This caused a large set of inequality constraints to be active in the solver, leading to performance issues. This PR restores the behaviour prior to #3182.

@daljit46 daljit46 self-assigned this Jan 22, 2026
@daljit46 daljit46 added the bug label Jan 22, 2026
@daljit46 daljit46 requested a review from Lestropie January 22, 2026 12:13
@github-actions
Copy link

clang-tidy review says "All clean, LGTM! 👍"

We were mistakenly setting the last `num_ineq` entries active, while
we should be doing the opposite: set the last `num_eq` entries active.
@github-actions
Copy link

clang-tidy review says "All clean, LGTM! 👍"

@bjeurissen
Copy link
Member

Well spotted! Was it only a matter of speed or did it also change the outcome? I would suspect the former as the tests still passed.

@jdtournier
Copy link
Member

As far as I can tell, that basically meant it was initialised with all constraints active, and since constraints are added/removed one at a time, this means it takes a lot longer to converge to the solution (which will typically have only a handful of constraints active). That explains the longer runtime, and why all the tests passed. Nice detective work!

Copy link
Member

@jdtournier jdtournier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix looks good to me.

@jdtournier jdtournier merged commit 6690c05 into dev Jan 23, 2026
6 checks passed
@jdtournier jdtournier deleted the fix_icls branch January 23, 2026 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants