Expose diving hyper parameters#1364
Open
nguidotti wants to merge 5 commits into
Open
Conversation
…udo_costs, worker and diving_heuristics. Signed-off-by: Nicolas L. Guidotti <nguidotti@nvidia.com>
nguidotti
added
non-breaking
📝 WalkthroughWalkthroughThis PR centralizes MIP diving hyperparameters into a templated ChangesDiving Heuristic Parameters
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 3
🧹 Nitpick comments (1)
cpp/include/cuopt/linear_programming/mip/diving_hyper_params.hpp (1)
8-8: ⚡ Quick winUse a macro guard here instead of only
#pragma once.The new C++ header does not follow the repository header-guard convention. Please add a normal
#defineguard and keep#pragma onceonly if you want it as a secondary optimization.As per coding guidelines, "Use
#defineguards on C++ headers".🤖 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/include/cuopt/linear_programming/mip/diving_hyper_params.hpp` at line 8, The header currently only has `#pragma once`; add a standard include guard macro around the file to follow project convention: introduce a unique macro like CPP_INCLUDE_CUOPT_LINEAR_PROGRAMMING_MIP_DIVING_HYPER_PARAMS_HPP (or similar uppercase, path-based name) with `#ifndef ... `#define` ...` at the top and a matching `#endif` at the bottom of diving_hyper_params.hpp, keeping `#pragma once` if you want it as a secondary optimization; ensure the macro name is unique and consistent with other headers.
🤖 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 1758-1760: The code unconditionally sets
diving_settings.guided_diving = 1 before calling
worker->calculate_num_diving_workers(...), overwriting the caller's intended
mode; change this to preserve and reuse the original mode by assigning
diving_settings.guided_diving = settings_.diving_settings.guided_diving (so
guided_diving keeps -1 for automatic or 1 for enabled) before invoking
worker->calculate_num_diving_workers.
In `@cpp/src/branch_and_bound/diving_heuristics.hpp`:
- Around line 42-43: The declaration of is_search_strategy_enabled in
diving_heuristics.hpp uses the type mip_diving_hyper_params_t but the header
doesn't directly include the header that defines that type; make the header
self-contained by adding an `#include` for the header that introduces
mip_diving_hyper_params_t (the file that defines the mip_diving_hyper_params_t
type) to diving_heuristics.hpp so the declaration compiles without relying on
transitive includes.
In `@cpp/src/math_optimization/solver_settings.cu`:
- Around line 118-119: The registration passes
&mip_settings.diving_params.iteration_limit_factor (declared as double in
diving_hyper_params.hpp) into a parameter_info_t<f_t> (used by
solver_settings_t<int, float>), causing a double* vs float* mismatch; fix by
changing the type of iteration_limit_factor in diving_hyper_params.hpp to f_t so
it matches parameter_info_t<f_t>, or alternatively add a specialized
registration path that accepts a double-typed parameter (i.e., a
parameter_info_t<double> entry) for
CUOPT_MIP_HYPER_DIVING_ITERATION_LIMIT_FACTOR and ensure solver_settings
registration logic handles that specialization.
---
Nitpick comments:
In `@cpp/include/cuopt/linear_programming/mip/diving_hyper_params.hpp`:
- Line 8: The header currently only has `#pragma once`; add a standard include
guard macro around the file to follow project convention: introduce a unique
macro like CPP_INCLUDE_CUOPT_LINEAR_PROGRAMMING_MIP_DIVING_HYPER_PARAMS_HPP (or
similar uppercase, path-based name) with `#ifndef ... `#define` ...` at the top
and a matching `#endif` at the bottom of diving_hyper_params.hpp, keeping
`#pragma once` if you want it as a secondary optimization; ensure the macro name
is unique and consistent with other headers.
🪄 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: Enterprise
Run ID: 877a2203-2924-4ddf-bcb2-d88a2e40efd0
📒 Files selected for processing (11)
cpp/include/cuopt/linear_programming/constants.hcpp/include/cuopt/linear_programming/mip/diving_hyper_params.hppcpp/include/cuopt/linear_programming/mip/solver_settings.hppcpp/src/branch_and_bound/branch_and_bound.cppcpp/src/branch_and_bound/diving_heuristics.cppcpp/src/branch_and_bound/diving_heuristics.hppcpp/src/branch_and_bound/pseudo_costs.hppcpp/src/branch_and_bound/worker.hppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/math_optimization/solver_settings.cucpp/src/mip_heuristics/solver.cu
…derabbit comments. Signed-off-by: Nicolas L. Guidotti <nguidotti@nvidia.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cpp/src/branch_and_bound/branch_and_bound.cpp (1)
1755-1761:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPreserve the original guided-diving mode when re-enabling.
Line 1758 hardcodes
diving_settings.guided_diving = 1, but the original setting might have been-1(automatic) rather than1(enabled). Restoring the original value fromsettings_.diving_settings.guided_divingpreserves the caller's intended mode.Suggested fix
- diving_settings.guided_diving = 1; + diving_settings.guided_diving = settings_.diving_settings.guided_diving;🤖 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 1755 - 1761, The code currently forces diving_settings.guided_diving = 1 when re-enabling guided diving; instead restore the original mode from settings_.diving_settings.guided_diving so automatic (-1) vs enabled (1) is preserved. Update the block that checks diving_worker_pool_.size() and settings_.diving_settings.guided_diving to assign diving_settings.guided_diving = settings_.diving_settings.guided_diving (not literal 1) before calling worker->calculate_num_diving_workers(bfs_worker_pool_.size(), diving_worker_pool_.size(), diving_settings), keeping the has_solver_space_incumbent() check unchanged.
🤖 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.
Duplicate comments:
In `@cpp/src/branch_and_bound/branch_and_bound.cpp`:
- Around line 1755-1761: The code currently forces diving_settings.guided_diving
= 1 when re-enabling guided diving; instead restore the original mode from
settings_.diving_settings.guided_diving so automatic (-1) vs enabled (1) is
preserved. Update the block that checks diving_worker_pool_.size() and
settings_.diving_settings.guided_diving to assign diving_settings.guided_diving
= settings_.diving_settings.guided_diving (not literal 1) before calling
worker->calculate_num_diving_workers(bfs_worker_pool_.size(),
diving_worker_pool_.size(), diving_settings), keeping the
has_solver_space_incumbent() check unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 69c1430e-729e-4b5e-ab18-aa8abd5151ec
📒 Files selected for processing (10)
cpp/include/cuopt/linear_programming/constants.hcpp/include/cuopt/linear_programming/mip/diving_hyper_params.hppcpp/include/cuopt/linear_programming/mip/heuristics_hyper_params.hppcpp/include/cuopt/linear_programming/mip/solver_settings.hppcpp/src/branch_and_bound/branch_and_bound.cppcpp/src/branch_and_bound/diving_heuristics.hppcpp/src/branch_and_bound/worker.hppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/math_optimization/solver_settings.cucpp/tests/mip/heuristics_hyper_params_test.cu
✅ Files skipped from review due to trivial changes (1)
- cpp/src/math_optimization/solver_settings.cu
🚧 Files skipped from review as they are similar to previous changes (5)
- cpp/src/branch_and_bound/diving_heuristics.hpp
- cpp/include/cuopt/linear_programming/mip/solver_settings.hpp
- cpp/include/cuopt/linear_programming/constants.h
- cpp/src/dual_simplex/simplex_solver_settings.hpp
- cpp/src/branch_and_bound/worker.hpp
akifcorduk
reviewed
Jun 2, 2026
Signed-off-by: Nicolas L. Guidotti <nguidotti@nvidia.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
cpp/include/cuopt/linear_programming/mip/diving_hyper_params.hpp (2)
22-26: ⚡ Quick winAdd per-member documentation for heuristic enable flags.
The tri-state semantics (-1 automatic, 0 disabled, 1 enabled) are documented only in a single-line comment. For a public API, consider adding inline documentation for each flag explaining what "automatic" mode does for that specific heuristic.
📝 Example documentation pattern
- // -1 automatic, 0 disabled, 1 enabled - i_t line_search_diving = -1; + /// Enable line-search diving heuristic: -1 (automatic, enabled based on problem characteristics), 0 (disabled), 1 (enabled). + i_t line_search_diving = -1;As per coding guidelines, public C++ headers should document algorithm parameters clearly.
🤖 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/include/cuopt/linear_programming/mip/diving_hyper_params.hpp` around lines 22 - 26, Add inline Doxygen-style comments for each public member (line_search_diving, pseudocost_diving, guided_diving, coefficient_diving) that state the tri-state semantics (-1 = automatic, 0 = disabled, 1 = enabled) and briefly explain what "automatic" chooses for that specific heuristic (e.g., select based on problem size/LP relaxation info, use historical pseudocosts, enable guided branching when strong inference exists, or pick coefficient-based rules respectively). Place these short comments immediately above each member declaration in diving_hyper_params.hpp so the public API documents behavior per-flag.
12-21: ⚡ Quick winDocument template parameters.
The template parameters
i_tandf_tare not documented. Consider adding@tparamtags to clarify expected types and constraints (e.g.,i_tmust be a signed integer type to support the -1 automatic mode).📝 Suggested documentation enhancement
/** * `@brief` Tuning knobs for the dual-simplex diving heuristics used in MIP B&B. * + * `@tparam` i_t Signed integer type for counts, limits, and tri-state flags. + * `@tparam` f_t Floating-point type for scaling factors. + * * Used directly as simplex_solver_settings_t::diving_settings and copied intoAs per coding guidelines, public C++ headers should document parameters and types for API clarity.
🤖 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/include/cuopt/linear_programming/mip/diving_hyper_params.hpp` around lines 12 - 21, Add Doxygen `@tparam` documentation for the template parameters on the mip_diving_hyper_params_t template: document i_t as the integer index/size type (must be a signed integer type because -1 is used for "automatic" mode) and document f_t as the floating-point type used for numeric thresholds/weights (e.g., double/float). Place the `@tparam` entries in the existing comment block immediately above the template<typename i_t, typename f_t> struct mip_diving_hyper_params_t so the API docs clearly state the expected types and any constraints.
🤖 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/include/cuopt/linear_programming/mip/diving_hyper_params.hpp`:
- Around line 22-26: Add inline Doxygen-style comments for each public member
(line_search_diving, pseudocost_diving, guided_diving, coefficient_diving) that
state the tri-state semantics (-1 = automatic, 0 = disabled, 1 = enabled) and
briefly explain what "automatic" chooses for that specific heuristic (e.g.,
select based on problem size/LP relaxation info, use historical pseudocosts,
enable guided branching when strong inference exists, or pick coefficient-based
rules respectively). Place these short comments immediately above each member
declaration in diving_hyper_params.hpp so the public API documents behavior
per-flag.
- Around line 12-21: Add Doxygen `@tparam` documentation for the template
parameters on the mip_diving_hyper_params_t template: document i_t as the
integer index/size type (must be a signed integer type because -1 is used for
"automatic" mode) and document f_t as the floating-point type used for numeric
thresholds/weights (e.g., double/float). Place the `@tparam` entries in the
existing comment block immediately above the template<typename i_t, typename
f_t> struct mip_diving_hyper_params_t so the API docs clearly state the expected
types and any constraints.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c64e4a8f-3405-47c1-a6e9-11eff3ac628b
📒 Files selected for processing (2)
cpp/include/cuopt/linear_programming/mip/diving_hyper_params.hppcpp/src/branch_and_bound/diving_heuristics.hpp
🚧 Files skipped from review as they are similar to previous changes (1)
- cpp/src/branch_and_bound/diving_heuristics.hpp
akifcorduk
approved these changes
Jun 5, 2026
aliceb-nv
approved these changes
Jun 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Similar to #993, this PR exposes the diving hyper parameters to
cuopt_cliand to the public API in order to be easier to tune. They are hidden by default incuopt_cli.Checklist