Cpufj cut bursts#12
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe pull request integrates CPU feasibility-jump (CPUFJ) support into the branch-and-bound root cut-passes phase. It introduces task-oriented APIs to create and run CPUFJ from host-level linear programs, refactors the root cut-pass loop to execute CPUFJ concurrently with cut passes, and adds work-unit limiting and improved callback context capture. ChangesRoot CPUFJ Integration
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
cpp/src/mip_heuristics/local_search/local_search.cu (1)
127-136: ⚡ Quick winKeep this scratch callback self-contained.
This background callback only needs
context.problem_ptr, but it now capturesthis. That adds an avoidable lifetime dependency onlocal_search_tduring shutdown; the sibling scratch callback in this file already uses the safer by-valueproblem_ptrcapture.♻️ Suggested change
- scratch_cpu_fj_on_lp_opt.fj_cpu->improvement_callback = - [&population, this](f_t obj, const std::vector<f_t>& h_vec, double /*work_units*/) { + scratch_cpu_fj_on_lp_opt.fj_cpu->improvement_callback = + [&population, problem_ptr = context.problem_ptr]( + f_t obj, const std::vector<f_t>& h_vec, double /*work_units*/) { population.add_external_solution(h_vec, obj, solution_origin_t::CPUFJ); if (obj < local_search_best_obj) { CUOPT_LOG_DEBUG("******* New local search best obj %g, best overall %g", - context.problem_ptr->get_user_obj_from_solver_obj(obj), - context.problem_ptr->get_user_obj_from_solver_obj( + problem_ptr->get_user_obj_from_solver_obj(obj), + problem_ptr->get_user_obj_from_solver_obj( population.is_feasible() ? population.best_feasible().get_objective() : std::numeric_limits<f_t>::max())); local_search_best_obj = obj; } };🤖 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/mip_heuristics/local_search/local_search.cu` around lines 127 - 136, Change the lambda capture to avoid capturing this; capture only the needed symbols explicitly: capture population by reference (&population), capture local_search_best_obj by reference (&local_search_best_obj) if you need to update it (or capture a pointer to it), and capture problem_ptr by value (problem_ptr = context.problem_ptr) so the callback no longer depends on the local_search_t lifetime; update the capture list for the lambda assigned to scratch_cpu_fj_on_lp_opt.fj_cpu->improvement_callback accordingly.
🤖 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 2231-2237: The callback root_cut_cpufj_improvement_callback is
passing a solver-space prefix directly into set_new_solution by slicing
assignment with original_problem_.num_cols; instead translate the solver-space
CPUFJ solution back to the user-space layout used by set_new_solution (use the
existing uncrush_primal_solution mapping that was used to produce the CPUFJ
vector) and then call set_new_solution with that translated user_assignment, or
alternatively add a solver-space ingestion path (e.g. a new
set_new_solution_from_solver or similar) and use it here so the incumbent is
validated/updated against the correct assignment.
In `@cpp/src/mip_heuristics/feasibility_jump/fj_cpu.cu`:
- Around line 1456-1460: The tolerances struct used for the host-LP CPUFJ path
is missing initialization of relative_tolerance, causing
get_corrected_tolerance() (used in the CPUFJ loop) to enforce different
thresholds than problem_t/fj_cpu.view.pb; set tolerances.relative_tolerance =
settings.relative_tol (or the appropriate settings field for relative tolerance)
alongside the existing assignments so the full tolerance mapping matches the
regular path and scaled rows use the same feasibility threshold.
---
Nitpick comments:
In `@cpp/src/mip_heuristics/local_search/local_search.cu`:
- Around line 127-136: Change the lambda capture to avoid capturing this;
capture only the needed symbols explicitly: capture population by reference
(&population), capture local_search_best_obj by reference
(&local_search_best_obj) if you need to update it (or capture a pointer to it),
and capture problem_ptr by value (problem_ptr = context.problem_ptr) so the
callback no longer depends on the local_search_t lifetime; update the capture
list for the lambda assigned to
scratch_cpu_fj_on_lp_opt.fj_cpu->improvement_callback accordingly.
🪄 Autofix (Beta)
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: Pro
Run ID: 21466f26-3d91-490f-8100-8fe0859b7bab
📒 Files selected for processing (5)
cpp/src/branch_and_bound/branch_and_bound.cppcpp/src/mip_heuristics/feasibility_jump/cpu_fj_thread.cuhcpp/src/mip_heuristics/feasibility_jump/fj_cpu.cucpp/src/mip_heuristics/feasibility_jump/fj_cpu.cuhcpp/src/mip_heuristics/local_search/local_search.cu
1 participant
Dummy PR to get coderabbit to perform a review.
Description
Issue
Checklist