Skip to content

Make sure all similar magnitude values are on one side - #3575

Merged
fwesselm merged 5 commits into
latestfrom
change-order-dual-fix-feastol-check
Nov 9, 2026
Merged

fwesselm merged 5 commits into
latestfrom
change-order-dual-fix-feastol-check

Conversation

@Opt-Mucca

@Opt-Mucca Opt-Mucca commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

Description

This fixes #3561

The issue: We were comparing x <= x + feastol, where feastol was being swallowed by a large x. I've rearranged it to x - x <= feastol. The numerics should be a bit friendlier, and at least for this instance, it no longer returns an incorrect result. Edit: "this instance" is the one found in the issue

Checklist

  • I have read the contributing guidelines
  • This PR targets the latest branch
  • Tests are passing
  • Documentation was updated where relevant
  • This PR is not primarily AI-generated (per the AI contributions policy in CONTRIBUTING.md)

Related issue

Closes #3561

@codecov

codecov Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.71%. Comparing base (a9c6bed) to head (8ac0444).
⚠️ Report is 20 commits behind head on latest.

Additional details and impacted files
@@           Coverage Diff           @@
##           latest    #3575   +/-   ##
=======================================
  Coverage   75.70%   75.71%           
=======================================
  Files         461      461           
  Lines      114020   114012    -8     
  Branches    17933    17939    +6     
=======================================
- Hits        86322    86319    -3     
+ Misses      27410    27405    -5     
  Partials      288      288           

☔ View full report in Codecov by Harness.
📢 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.

@DoumarCharles

Copy link
Copy Markdown

Thanks for fixing this!

I initially wondered whether simply checking M <= b would be better. It leaves the full feasibility tolerance as a margin before the row could be considered violated. But I see the reasoning behind M - b <= feastol. It keeps the tolerance used throughout this code, and putting M and b on the same side avoids feastol being rounded away in b + feastol when b is large.

The change makes sense to me, and I'll follow the same logic in anything I suggest from now on.

@fwesselm
fwesselm merged commit 98391ab into latest Nov 9, 2026
631 of 632 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants