Root Heuristics (RINS, CPU FJ) - #1699
Conversation
…rom submip. Signed-off-by: Nicolas L. Guidotti <nguidotti@nvidia.com>
Signed-off-by: Nicolas L. Guidotti <nguidotti@nvidia.com>
Signed-off-by: Nicolas L. Guidotti <nguidotti@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
📝 WalkthroughWalkthroughThe PR adds configurable DFS backtracking for SubMIP feasibility searches. It refactors SubMIP and RINS objective handling, worker-state reuse, postsolve processing, and asynchronous root heuristic scheduling. It also initializes new feasibility-jump constraint weights to one. ChangesRoot heuristic execution
Feasibility-jump weight initialization
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
cpp/src/branch_and_bound/branch_and_bound.cpp (1)
2814-2825: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
heuristics.emplace_backruns before both capacity checks.Line 2825 appends a
root_heuristics_tunconditionally. The RINS check at line 2827 and the CPU FJ check at line 2853 can both fail, for example when there is no incumbent and*worker_countreachedsettings_.num_threads - 1between the entry check and line 2853. The list then holds an element with nosubmip_worker_and no started FJ worker.The element is harmless and the list is bounded by
settings_.max_cut_passes, so this is not a correctness defect. Move theemplace_backafter the first successful capacity check to keep the list meaningful.🤖 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/src/branch_and_bound/branch_and_bound.cpp` around lines 2814 - 2825, Move the heuristics.emplace_back call and references to the created heuristic in launch_root_heuristics so they occur only after the first RINS capacity check succeeds. Ensure failed RINS and CPU FJ capacity checks do not append an uninitialized root_heuristics_t entry, while preserving the existing heuristic setup and execution behavior.
🤖 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/branch_and_bound/branch_and_bound.cpp`:
- Around line 658-664: In the root-heuristic solution handling at
cpp/src/branch_and_bound/branch_and_bound.cpp:658-664, lock mutex_original_lp_
before accessing original_lp_ for compute_objective and handle leaf_sol values
narrower than original_lp_.num_cols without out-of-bounds access. In rins at
cpp/src/branch_and_bound/branch_and_bound.cpp:2579-2593, snapshot var_types_
under mutex_original_lp_ once at the start into worker-local storage, then pass
that snapshot to fractional_variables instead of repeatedly reading the shared
member.
In `@cpp/src/mip_heuristics/feasibility_jump/fj_cpu_worker.cuh`:
- Around line 50-52: Add a direct include for utilities/omp_helpers.hpp in
fj_cpu_worker.cuh so the omp_atomic_t type used by run_async is declared
independently of transitive includes.
In `@cpp/src/mip_heuristics/root_heuristics.hpp`:
- Around line 29-37: Update the RINS fixing loop condition in
root_heuristics_t::rins() to also require !worker->halt, while preserving the
existing solver_status_ and is_running_ checks. This ensures the loop exits when
root_heuristics_t::stop() signals the worker before the heuristic state is
destroyed.
---
Nitpick comments:
In `@cpp/src/branch_and_bound/branch_and_bound.cpp`:
- Around line 2814-2825: Move the heuristics.emplace_back call and references to
the created heuristic in launch_root_heuristics so they occur only after the
first RINS capacity check succeeds. Ensure failed RINS and CPU FJ capacity
checks do not append an uninitialized root_heuristics_t entry, while preserving
the existing heuristic setup and execution behavior.
🪄 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: c6a6266b-2ede-44d7-b8c8-d8c265d981ac
📒 Files selected for processing (12)
cpp/include/cuopt/mathematical_optimization/mip/submip_hyper_params.hppcpp/src/branch_and_bound/branch_and_bound.cppcpp/src/branch_and_bound/branch_and_bound.hppcpp/src/branch_and_bound/worker.hppcpp/src/dual_simplex/solve.cppcpp/src/dual_simplex/solve.hppcpp/src/mip_heuristics/feasibility_jump/feasibility_jump.cucpp/src/mip_heuristics/feasibility_jump/fj_cpu.cucpp/src/mip_heuristics/feasibility_jump/fj_cpu_worker.cuhcpp/src/mip_heuristics/presolve/third_party_presolve.cppcpp/src/mip_heuristics/presolve/third_party_presolve.hppcpp/src/mip_heuristics/root_heuristics.hpp
CI Test Summary⏭️ All 5 test job(s) skipped. |
set_solution_from_submip(the objective was on the submip space, which is not comparable with the solution space of the B&B).Checklist