Skip to content

Fix GF2 incorrectly treating some one-sided rows as equality - #1707

Merged
rapids-bot[bot] merged 1 commit into
NVIDIA:mainfrom
aliceb-nv:gf2-eq-fix
Aug 13, 2026
Merged

Fix GF2 incorrectly treating some one-sided rows as equality#1707
rapids-bot[bot] merged 1 commit into
NVIDIA:mainfrom
aliceb-nv:gf2-eq-fix

Conversation

@aliceb-nv

Copy link
Copy Markdown
Contributor

The GF2 presolver was using the wrong test to check if a Papilo row was an equality. This PR fixes this by checking the proper flags. The timeout for one GF2 test was also very slightly increased to be more robust w.r.t. CI.

Description

Issue

Checklist

  • I am familiar with the Contributing Guidelines.
  • Testing
    • New or existing tests cover these changes
    • Added tests
    • Created an issue to follow-up
    • NA
  • Documentation
    • The documentation is up to date with these changes
    • Added new documentation
    • NA

@aliceb-nv aliceb-nv added this to the 26.10 milestone Aug 12, 2026
@aliceb-nv
aliceb-nv requested a review from a team as a code owner August 12, 2026 20:22
@aliceb-nv aliceb-nv added bug Something isn't working non-breaking Introduces a non-breaking change labels Aug 12, 2026
@copy-pr-bot

copy-pr-bot Bot commented Aug 12, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@aliceb-nv

Copy link
Copy Markdown
Contributor Author

/ok to test 7558040

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

GF2 presolve now supports pinned equations and one-sided rows, validates their integer values, retains full right-hand sides, and applies parity conversion when building the binary system. GF2 termination tests use longer time limits.

Changes

GF2 presolve row handling

Layer / File(s) Summary
Pinned-row validation
cpp/src/mip_heuristics/presolve/gf2_presolve.hpp, cpp/src/mip_heuristics/presolve/gf2_presolve.cpp
GF2 presolve derives fixed row values from equality flags or exact one-sided activities. It rejects redundant, nonfinite, noninteger, and oversized values.
Parity conversion and reconstruction
cpp/src/mip_heuristics/presolve/gf2_presolve.cpp, cpp/tests/mip/termination_test.cu
GF2 constraints retain full integer right-hand sides, convert them to parity when building the binary system, and use them for key-variable reconstruction. GF2 test time limits increase from 0.5 to 1.5 seconds.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: bubullzz, nguidotti, mlubin

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the GF2 presolver issue and the fix for incorrect one-sided row classification.
Description check ✅ Passed The description accurately explains the equality-row detection fix and the test timeout increase.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cpp/src/mip_heuristics/presolve/gf2_presolve.cpp`:
- Around line 203-217: Add regression cases in gf2_presolve_test.cpp covering
pinned_row_value for RHS-pinned rows, LHS-pinned rows, and equality rows, plus a
non-pinned row that is rejected. Assert the expected pinned value or rejection
behavior while preserving the existing equality-row coverage.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: cc535af1-061f-48a5-aa24-d9b9129008a8

📥 Commits

Reviewing files that changed from the base of the PR and between 6f164ae and 7558040.

📒 Files selected for processing (3)
  • cpp/src/mip_heuristics/presolve/gf2_presolve.cpp
  • cpp/src/mip_heuristics/presolve/gf2_presolve.hpp
  • cpp/tests/mip/termination_test.cu

Comment thread cpp/src/mip_heuristics/presolve/gf2_presolve.cpp
@aliceb-nv

Copy link
Copy Markdown
Contributor Author

/merge

@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown

CI Test Summary

✅ All 31 test job(s) passed.

@rapids-bot
rapids-bot Bot merged commit 6a5dcbf into NVIDIA:main Aug 13, 2026
148 of 153 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants