fix: incorrect GF2 presolve constraint addressing

[aliceb-nv]

The GF2 presolver pass was addressing its GF2-space columns with original problem space column indices, which would trigger OoBs when GF2 structures were recognized on a problem that isn't purely GF2.

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]

/ok to test bf007d7

[nguidotti]

Thanks for the quick fix!

[coderabbitai]

📝 Walkthrough

Walkthrough

Fixes row indexing in the GF(2) presolve matrix/vector construction to use the position within gf2_constraints rather than the original constraint index, and adds a GoogleTest case exercising this compact-index path.

Changes

GF2 Presolve Indexing Fix

Layer / File(s)	Summary
Fix GF(2) matrix/vector row indexing cpp/src/mip_heuristics/presolve/gf2_presolve.cpp	A and b are now filled using gf2_cstr_idx (position within gf2_constraints) instead of the original cons.cstr_idx, aligning row indexing with allocated matrix dimensions.
Regression test for compact constraint indices cpp/tests/mip/presolve_test.cu	Adds <algorithm>/<array> includes and a new test gf2_presolve.uses_compact_constraint_indices building a mixed packing/key/GF(2) CSR MIP, running mip::third_party_presolve_t, and asserting REDUCED status.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Test as presolve_test.cu
  participant Presolve as GF2Presolve::execute
  participant Matrix as A/b construction

  Test->>Presolve: run third_party_presolve_t on CSR MIP
  Presolve->>Matrix: iterate gf2_constraints
  Matrix->>Matrix: fill A[gf2_cstr_idx][...] and b[gf2_cstr_idx]
  Presolve-->>Test: return REDUCED status

Loading

Related PRs: None identified.

Suggested labels: bug, presolve, tests

Suggested reviewers: None specified.

Poem
A rabbit hopped through rows of GF(2),
Where indices once wandered, now aligned and true.
Compact and neat, each row finds its place,
A test stands guard to confirm the fix's grace.
Presolve REDUCED — hop, hop, hooray!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the main fix: correcting GF2 presolve constraint addressing.
Description check	✅ Passed	The description directly explains the indexing bug and the resulting out-of-bounds issue.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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]

🧹 Nitpick comments (1)

cpp/tests/mip/presolve_test.cu (1)

68-151: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Test only checks overall status; doesn't verify GF2 fixings themselves.

The test is well constructed to reproduce the reported bug: it mixes 128 packing rows and 12 key rows (raw indices 0-139) with 6 equality GF2 rows (raw indices 140-145) so the GF2 subset's original cstr_idx values (140-145) exceed gf2_constraints.size() (6), exactly the scenario that overflowed A/b before the fix.

However, the assertion only checks result.status == REDUCED, which just confirms presolve ran to completion without crashing/UB — it doesn't confirm the GF(2) system was solved correctly (i.e., that the 12 GF2/key variables actually got fixed to consistent values in result.reduced_problem). As per path instructions, cpp/tests should focus on "Numerical correctness validation (not just 'runs without error')". Consider additionally asserting that the GF2 bin/key variable bounds collapsed (lower == upper) in the reduced problem to also catch correctness regressions in the fixing logic, not just the addressing bug.

🤖 Prompt for 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.

In `@cpp/tests/mip/presolve_test.cu` around lines 68 - 151, The test in
gf2_presolve::uses_compact_constraint_indices only verifies that presolve
returns REDUCED, but it does not check that the GF(2) elimination actually fixed
the intended variables. Extend the assertions against result.reduced_problem to
confirm the GF2/key variables have collapsed bounds (lower == upper) after
third_party_presolve_t<int, double>::apply, using the existing x_offset,
y_offset, and num_key_vars / num_gf2_vars symbols to locate the affected
variables. Keep the current status check, but add numerical correctness checks
for the GF2-related variable fixings so the test validates the solving result,
not just that it ran successfully.

Source: Path instructions

🤖 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.

Nitpick comments:
In `@cpp/tests/mip/presolve_test.cu`:
- Around line 68-151: The test in gf2_presolve::uses_compact_constraint_indices
only verifies that presolve returns REDUCED, but it does not check that the
GF(2) elimination actually fixed the intended variables. Extend the assertions
against result.reduced_problem to confirm the GF2/key variables have collapsed
bounds (lower == upper) after third_party_presolve_t<int, double>::apply, using
the existing x_offset, y_offset, and num_key_vars / num_gf2_vars symbols to
locate the affected variables. Keep the current status check, but add numerical
correctness checks for the GF2-related variable fixings so the test validates
the solving result, not just that it ran successfully.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: a0e0b65b-a359-4ac3-979d-8d8e730a4d2f

📥 Commits

Reviewing files that changed from the base of the PR and between fe7348a and bf007d7.

📒 Files selected for processing (2)

• cpp/src/mip_heuristics/presolve/gf2_presolve.cpp
• cpp/tests/mip/presolve_test.cu

[github-actions]

CI Test Summary

✅ All 31 test job(s) passed.

[aliceb-nv]

/merge