Disable barrier and cuts tests#968
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughSeveral test cases were disabled: specific parameter combinations in Python linear-programming tests were marked skipped via pytest.param, and two C++ GoogleTest cases were renamed with the DISABLED_ prefix to prevent execution. No other functional code changes were made. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@python/cuopt/cuopt/tests/linear_programming/test_python_API.py`:
- Around line 515-525: The unconditional skips on the pytest.param entries for
the barrier-augmented configurations (the pytest.param with keys CUOPT_FOLDING,
CUOPT_DUALIZE, CUOPT_ORDERING, CUOPT_AUGMENTED and the other similar
pytest.param blocks) must be made trackable and time-bounded: replace the bare
pytest.mark.skip(...) with a skip reason that includes a tracker/issue ID (e.g.
"CUOPT-XXXXX") and a clear re-enable condition ("re-enable when barrier
initial-point fix is in the build"), add a TODO comment referencing the issue ID
next to the pytest.param, and (where appropriate) consider using
pytest.mark.xfail or pytest.skipif with a short expiry date or env-flag to avoid
permanent loss of numerical-correctness coverage; also ensure the tests validate
numerical correctness of the optimization results rather than only running
without error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 47726764-8795-42ce-bb01-2286bed18e31
📒 Files selected for processing (1)
python/cuopt/cuopt/tests/linear_programming/test_python_API.py
| pytest.param( | ||
| "mixed", | ||
| { | ||
| CUOPT_FOLDING: 1, | ||
| CUOPT_DUALIZE: 0, | ||
| CUOPT_ORDERING: -1, | ||
| CUOPT_AUGMENTED: 1, | ||
| }, | ||
| marks=pytest.mark.skip( | ||
| reason="Barrier augmented-system numerical issue; re-enable when barrier initial-point fix is in the build" | ||
| ), |
There was a problem hiding this comment.
Avoid indefinite untracked skips for barrier coverage.
At Line 515, Line 563, and Line 624, these cases are now unconditionally skipped, which drops numerical-correctness coverage for key barrier configurations. Please make the disablement explicitly time-bounded/trackable (issue ID + re-enable condition) so this does not become permanent technical debt.
Suggested change
+_BARRIER_TEMP_DISABLE_MARK = pytest.mark.xfail(
+ reason=(
+ "TODO(#<issue-id>): Barrier augmented-system numerical issue. "
+ "Re-enable after barrier initial-point fix lands in build."
+ ),
+ run=False,
+)
+
pytest.param(
"mixed",
{
CUOPT_FOLDING: 1,
CUOPT_DUALIZE: 0,
CUOPT_ORDERING: -1,
CUOPT_AUGMENTED: 1,
},
- marks=pytest.mark.skip(
- reason="Barrier augmented-system numerical issue; re-enable when barrier initial-point fix is in the build"
- ),
+ marks=_BARRIER_TEMP_DISABLE_MARK,
),
@@
pytest.param(
"augmented_system",
{
CUOPT_AUGMENTED: 1,
},
- marks=pytest.mark.skip(
- reason="Barrier augmented-system numerical issue; re-enable when barrier initial-point fix is in the build"
- ),
+ marks=_BARRIER_TEMP_DISABLE_MARK,
),
@@
pytest.param(
"combo3_with_dual_init",
{
CUOPT_AUGMENTED: 1,
CUOPT_BARRIER_DUAL_INITIAL_POINT: 1,
CUOPT_ELIMINATE_DENSE_COLUMNS: True,
},
- marks=pytest.mark.skip(
- reason="Barrier augmented-system numerical issue; re-enable when barrier initial-point fix is in the build"
- ),
+ marks=_BARRIER_TEMP_DISABLE_MARK,
),Also applies to: 563-570, 624-633
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@python/cuopt/cuopt/tests/linear_programming/test_python_API.py` around lines
515 - 525, The unconditional skips on the pytest.param entries for the
barrier-augmented configurations (the pytest.param with keys CUOPT_FOLDING,
CUOPT_DUALIZE, CUOPT_ORDERING, CUOPT_AUGMENTED and the other similar
pytest.param blocks) must be made trackable and time-bounded: replace the bare
pytest.mark.skip(...) with a skip reason that includes a tracker/issue ID (e.g.
"CUOPT-XXXXX") and a clear re-enable condition ("re-enable when barrier
initial-point fix is in the build"), add a TODO comment referencing the issue ID
next to the pytest.param, and (where appropriate) consider using
pytest.mark.xfail or pytest.skipif with a short expiry date or env-flag to avoid
permanent loss of numerical-correctness coverage; also ensure the tests validate
numerical correctness of the optimization results rather than only running
without error.
|
/merge |
Description
These tests are flaky and are disabled, and these will be moved to C layer later.
Checklist