update solver_settings#840
Merged
rapids-bot[bot] merged 13 commits intoNVIDIA:release/26.02from Feb 10, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a public accessor returning all solver parameter names; removes the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
python/cuopt_server/cuopt_server/utils/linear_programming/solver.py (1)
321-326:⚠️ Potential issue | 🟠 Major
set_mip_callback(None)appendsNoneto the callbacks list.When
intermediate_sender is None,callbackisNone(line 324). Callingsolver_settings.set_mip_callback(None)appendsNonetoself.mip_callbacks(line 262 ofsolver_settings.py). Downstream when these callbacks are iterated and passed to the C++ layer, aNoneentry could cause a crash or unexpected behavior.Guard the call:
Suggested fix
callback = ( CustomGetSolutionCallback(intermediate_sender, reqId) if intermediate_sender is not None else None ) - solver_settings.set_mip_callback(callback) + if callback is not None: + solver_settings.set_mip_callback(callback)
🤖 Fix all issues with AI agents
In `@cpp/src/math_optimization/solver_settings.cu`:
- Around line 109-110: The CUOPT_PRESOLVE parameter was accidentally changed
back to a bool entry, breaking the presolver_t enum and Python
set_parameter(int) callers; restore CUOPT_PRESOLVE as an integer/enum parameter
(not bool) by changing the entries that reference CUOPT_PRESOLVE in
solver_settings.cu so they point at the presolve/presolve fields typed as
presolver_t (or an int) and are registered in the int_parameters table (not
bool_parameters); ensure symbols referenced are CUOPT_PRESOLVE,
pdlp_settings.presolve, mip_settings.presolve, presolver_t and that Python
set_parameter(name, int) and downstream consumers (CLI, server) use the enum
integer values (Default/None/Papilo/PSLP) consistently.
In `@python/cuopt_server/cuopt_server/utils/linear_programming/solver.py`:
- Around line 126-134: The loop over solver_params can raise AttributeError when
a C++ parameter name isn't present on SolverConfig or SolverConfig.tolerances;
change the getattr calls used when reading param_value in that loop to provide a
safe default (e.g., None) so missing attributes return None instead of raising,
i.e., use getattr(solver_config.tolerances, param, None) for tolerance names and
getattr(solver_config, param, None) for others before calling
solver_settings.set_parameter; keep the existing check that skips None/empty
values so only present parameters are set.
In `@python/cuopt/cuopt/linear_programming/solver_settings/solver_settings.py`:
- Around line 282-293: The toDict method currently only converts positive
infinity to None (param_value == float("inf")), leaving float("-inf") unchanged;
update to detect any infinity by using math.isinf on values returned from
get_parameter and set param_value to None for both +inf and -inf (ensure to
import math), and guard the check so non-numeric parameter values don't
raise—apply this change inside the loop over solver_params where solver_config
and param_value are handled in toDict.
- Around line 178-180: The loop in set_optimality_tolerance currently updates
every param in solver_params that endswith("tolerance") and not
startswith("mip"), which incorrectly changes infeasibility detection thresholds
like primal_infeasible_tolerance and dual_infeasible_tolerance; change the logic
to only update the intended optimality tolerances (e.g., list the six optimality
keys explicitly) or at minimum exclude any parameter name containing
"infeasible"/"infeasibility" before assigning eps_optimal to
self.settings_dict[param]; ensure references to solver_params,
self.settings_dict, and eps_optimal remain unchanged and only the matching
condition is tightened.
🧹 Nitpick comments (4)
cpp/include/cuopt/linear_programming/solver_settings.hpp (1)
96-96:conston a return-by-value inhibits move semantics.Returning
const std::vector<std::string>by value prevents the compiler from applying move semantics at the call site. Drop theconstqualifier on the return type.Suggested fix
- const std::vector<std::string> get_parameter_names() const; + std::vector<std::string> get_parameter_names() const;cpp/src/math_optimization/solver_settings.cu (1)
442-459:get_parameter_names()returns duplicate names for shared parameters.Parameters like
CUOPT_TIME_LIMIT,CUOPT_NUM_GPUS,CUOPT_LOG_TO_CONSOLE,CUOPT_PRESOLVE,CUOPT_LOG_FILE,CUOPT_SOLUTION_FILE, andCUOPT_USER_PROBLEM_FILEeach appear twice (once forpdlp_settings, once formip_settings). The returned vector will contain these duplicates, which propagates to the Pythonsolver_paramslist causing redundant work in all loops that iterate over it. Consider deduplicating:Suggested fix — deduplicate using a set
template <typename i_t, typename f_t> -const std::vector<std::string> solver_settings_t<i_t, f_t>::get_parameter_names() const +std::vector<std::string> solver_settings_t<i_t, f_t>::get_parameter_names() const { - std::vector<std::string> parameter_names; - for (auto& param : int_parameters) { - parameter_names.push_back(param.param_name); - } - for (auto& param : float_parameters) { - parameter_names.push_back(param.param_name); - } - for (auto& param : bool_parameters) { - parameter_names.push_back(param.param_name); - } - for (auto& param : string_parameters) { - parameter_names.push_back(param.param_name); - } - return parameter_names; + std::set<std::string> seen; + std::vector<std::string> parameter_names; + auto insert = [&](const std::string& name) { + if (seen.insert(name).second) { parameter_names.push_back(name); } + }; + for (auto& param : int_parameters) { insert(param.param_name); } + for (auto& param : float_parameters) { insert(param.param_name); } + for (auto& param : bool_parameters) { insert(param.param_name); } + for (auto& param : string_parameters) { insert(param.param_name); } + return parameter_names; }python/cuopt/cuopt/linear_programming/solver/solver_parameters.pyx (1)
44-45:solver_paramswill be alist— consider converting to a unique ordered collection.Since
get_parameter_names()in C++ can return duplicate names (parameters shared between PDLP and MIP settings),solver_paramsmay contain duplicates. This list is used for membership tests (name not in solver_params) insolver_settings.pyand for iteration intoDict/set_optimality_tolerance. Consider deduplicating here (or in C++, per the comment onsolver_settings.cu):solver_params = list(dict.fromkeys(get_solver_parameter_names()))Also,
globals()["CUOPT_"+param.upper()] = paramdynamically sets module attributes that won't be visible to static analysis or IDE autocompletion. This is a trade-off of the dynamic approach — worth documenting.python/cuopt_server/cuopt_server/utils/linear_programming/solver.py (1)
135-136: Redundantsolver_configre-check and re-assignment.Lines 124–125 already check
LP_data.solver_config is not Noneand assignsolver_config. The identical check at lines 135–136 is unnecessary since we're still within the same function scope.Suggested fix — merge into one block
if LP_data.solver_config is not None: solver_config = LP_data.solver_config for param in solver_params: ... - if LP_data.solver_config is not None: - solver_config = LP_data.solver_config - try: lp_time_limit = float(os.environ.get("CUOPT_LP_TIME_LIMIT_SEC"))
python/cuopt/cuopt/linear_programming/solver_settings/solver_settings.py
Outdated
Show resolved
Hide resolved
Comment on lines
282
to
293
| def toDict(self): | ||
| time_limit = self.get_parameter(CUOPT_TIME_LIMIT) | ||
| if time_limit == float("inf"): | ||
| time_limit = None | ||
|
|
||
| solver_config = { | ||
| "tolerances": { | ||
| "absolute_dual_tolerance": self.get_parameter( | ||
| CUOPT_ABSOLUTE_DUAL_TOLERANCE | ||
| ), | ||
| "relative_dual_tolerance": self.get_parameter( | ||
| CUOPT_RELATIVE_DUAL_TOLERANCE | ||
| ), | ||
| "absolute_primal_tolerance": self.get_parameter( | ||
| CUOPT_ABSOLUTE_PRIMAL_TOLERANCE | ||
| ), | ||
| "relative_primal_tolerance": self.get_parameter( | ||
| CUOPT_RELATIVE_PRIMAL_TOLERANCE | ||
| ), | ||
| "absolute_gap_tolerance": self.get_parameter( | ||
| CUOPT_ABSOLUTE_GAP_TOLERANCE | ||
| ), | ||
| "relative_gap_tolerance": self.get_parameter( | ||
| CUOPT_RELATIVE_GAP_TOLERANCE | ||
| ), | ||
| "primal_infeasible_tolerance": self.get_parameter( | ||
| CUOPT_PRIMAL_INFEASIBLE_TOLERANCE | ||
| ), | ||
| "dual_infeasible_tolerance": self.get_parameter( | ||
| CUOPT_DUAL_INFEASIBLE_TOLERANCE | ||
| ), | ||
| "mip_integrality_tolerance": self.get_parameter( | ||
| CUOPT_MIP_INTEGRALITY_TOLERANCE | ||
| ), | ||
| "mip_absolute_gap": self.get_parameter(CUOPT_MIP_ABSOLUTE_GAP), | ||
| "mip_relative_gap": self.get_parameter(CUOPT_MIP_RELATIVE_GAP), | ||
| "mip_absolute_tolerance": self.get_parameter( | ||
| CUOPT_MIP_ABSOLUTE_TOLERANCE | ||
| ), | ||
| "mip_relative_tolerance": self.get_parameter( | ||
| CUOPT_MIP_RELATIVE_TOLERANCE | ||
| ), | ||
| }, | ||
| "infeasibility_detection": self.get_parameter( | ||
| CUOPT_INFEASIBILITY_DETECTION | ||
| ), | ||
| "time_limit": time_limit, | ||
| "iteration_limit": self.get_parameter(CUOPT_ITERATION_LIMIT), | ||
| "pdlp_solver_mode": self.get_parameter(CUOPT_PDLP_SOLVER_MODE), | ||
| "method": self.get_parameter(CUOPT_METHOD), | ||
| "presolve": self.get_parameter(CUOPT_PRESOLVE), | ||
| "dual_postsolve": self.get_parameter(CUOPT_DUAL_POSTSOLVE), | ||
| "mip_scaling": self.get_parameter(CUOPT_MIP_SCALING), | ||
| "mip_heuristics_only": self.get_parameter( | ||
| CUOPT_MIP_HEURISTICS_ONLY | ||
| ), | ||
| "num_cpu_threads": self.get_parameter(CUOPT_NUM_CPU_THREADS), | ||
| "num_gpus": self.get_parameter(CUOPT_NUM_GPUS), | ||
| "augmented": self.get_parameter(CUOPT_AUGMENTED), | ||
| "folding": self.get_parameter(CUOPT_FOLDING), | ||
| "dualize": self.get_parameter(CUOPT_DUALIZE), | ||
| "ordering": self.get_parameter(CUOPT_ORDERING), | ||
| "barrier_dual_initial_point": self.get_parameter( | ||
| CUOPT_BARRIER_DUAL_INITIAL_POINT | ||
| ), | ||
| "eliminate_dense_columns": self.get_parameter( | ||
| CUOPT_ELIMINATE_DENSE_COLUMNS | ||
| ), | ||
| "cudss_deterministic": self.get_parameter( | ||
| CUOPT_CUDSS_DETERMINISTIC | ||
| ), | ||
| "crossover": self.get_parameter(CUOPT_CROSSOVER), | ||
| "log_to_console": self.get_parameter(CUOPT_LOG_TO_CONSOLE), | ||
| "first_primal_feasible": self.get_parameter( | ||
| CUOPT_FIRST_PRIMAL_FEASIBLE | ||
| ), | ||
| "log_file": self.get_parameter(CUOPT_LOG_FILE), | ||
| "per_constraint_residual": self.get_parameter( | ||
| CUOPT_PER_CONSTRAINT_RESIDUAL | ||
| ), | ||
| "save_best_primal_so_far": self.get_parameter( | ||
| CUOPT_SAVE_BEST_PRIMAL_SO_FAR | ||
| ), | ||
| "solution_file": self.get_parameter(CUOPT_SOLUTION_FILE), | ||
| "strict_infeasibility": self.get_parameter( | ||
| CUOPT_STRICT_INFEASIBILITY | ||
| ), | ||
| "user_problem_file": self.get_parameter(CUOPT_USER_PROBLEM_FILE), | ||
| } | ||
| solver_config = {} | ||
| solver_config["tolerances"] = {} | ||
| for param in solver_params: | ||
| if param.endswith("tolerance"): | ||
| solver_config["tolerances"][param] = self.get_parameter(param) | ||
| else: | ||
| param_value = self.get_parameter(param) | ||
| if param_value == float("inf"): | ||
| param_value = None | ||
| solver_config[param] = param_value | ||
|
|
There was a problem hiding this comment.
toDict doesn't handle negative infinity.
Line 290 checks param_value == float("inf") and converts to None, but float("-inf") would pass through as-is, which may not serialize correctly (e.g., JSON doesn't support -Infinity). Consider checking with math.isinf(param_value) instead.
Suggested fix
+import math
+
...
- if param_value == float("inf"):
+ if isinstance(param_value, float) and math.isinf(param_value):
param_value = None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def toDict(self): | |
| time_limit = self.get_parameter(CUOPT_TIME_LIMIT) | |
| if time_limit == float("inf"): | |
| time_limit = None | |
| solver_config = { | |
| "tolerances": { | |
| "absolute_dual_tolerance": self.get_parameter( | |
| CUOPT_ABSOLUTE_DUAL_TOLERANCE | |
| ), | |
| "relative_dual_tolerance": self.get_parameter( | |
| CUOPT_RELATIVE_DUAL_TOLERANCE | |
| ), | |
| "absolute_primal_tolerance": self.get_parameter( | |
| CUOPT_ABSOLUTE_PRIMAL_TOLERANCE | |
| ), | |
| "relative_primal_tolerance": self.get_parameter( | |
| CUOPT_RELATIVE_PRIMAL_TOLERANCE | |
| ), | |
| "absolute_gap_tolerance": self.get_parameter( | |
| CUOPT_ABSOLUTE_GAP_TOLERANCE | |
| ), | |
| "relative_gap_tolerance": self.get_parameter( | |
| CUOPT_RELATIVE_GAP_TOLERANCE | |
| ), | |
| "primal_infeasible_tolerance": self.get_parameter( | |
| CUOPT_PRIMAL_INFEASIBLE_TOLERANCE | |
| ), | |
| "dual_infeasible_tolerance": self.get_parameter( | |
| CUOPT_DUAL_INFEASIBLE_TOLERANCE | |
| ), | |
| "mip_integrality_tolerance": self.get_parameter( | |
| CUOPT_MIP_INTEGRALITY_TOLERANCE | |
| ), | |
| "mip_absolute_gap": self.get_parameter(CUOPT_MIP_ABSOLUTE_GAP), | |
| "mip_relative_gap": self.get_parameter(CUOPT_MIP_RELATIVE_GAP), | |
| "mip_absolute_tolerance": self.get_parameter( | |
| CUOPT_MIP_ABSOLUTE_TOLERANCE | |
| ), | |
| "mip_relative_tolerance": self.get_parameter( | |
| CUOPT_MIP_RELATIVE_TOLERANCE | |
| ), | |
| }, | |
| "infeasibility_detection": self.get_parameter( | |
| CUOPT_INFEASIBILITY_DETECTION | |
| ), | |
| "time_limit": time_limit, | |
| "iteration_limit": self.get_parameter(CUOPT_ITERATION_LIMIT), | |
| "pdlp_solver_mode": self.get_parameter(CUOPT_PDLP_SOLVER_MODE), | |
| "method": self.get_parameter(CUOPT_METHOD), | |
| "presolve": self.get_parameter(CUOPT_PRESOLVE), | |
| "dual_postsolve": self.get_parameter(CUOPT_DUAL_POSTSOLVE), | |
| "mip_scaling": self.get_parameter(CUOPT_MIP_SCALING), | |
| "mip_heuristics_only": self.get_parameter( | |
| CUOPT_MIP_HEURISTICS_ONLY | |
| ), | |
| "num_cpu_threads": self.get_parameter(CUOPT_NUM_CPU_THREADS), | |
| "num_gpus": self.get_parameter(CUOPT_NUM_GPUS), | |
| "augmented": self.get_parameter(CUOPT_AUGMENTED), | |
| "folding": self.get_parameter(CUOPT_FOLDING), | |
| "dualize": self.get_parameter(CUOPT_DUALIZE), | |
| "ordering": self.get_parameter(CUOPT_ORDERING), | |
| "barrier_dual_initial_point": self.get_parameter( | |
| CUOPT_BARRIER_DUAL_INITIAL_POINT | |
| ), | |
| "eliminate_dense_columns": self.get_parameter( | |
| CUOPT_ELIMINATE_DENSE_COLUMNS | |
| ), | |
| "cudss_deterministic": self.get_parameter( | |
| CUOPT_CUDSS_DETERMINISTIC | |
| ), | |
| "crossover": self.get_parameter(CUOPT_CROSSOVER), | |
| "log_to_console": self.get_parameter(CUOPT_LOG_TO_CONSOLE), | |
| "first_primal_feasible": self.get_parameter( | |
| CUOPT_FIRST_PRIMAL_FEASIBLE | |
| ), | |
| "log_file": self.get_parameter(CUOPT_LOG_FILE), | |
| "per_constraint_residual": self.get_parameter( | |
| CUOPT_PER_CONSTRAINT_RESIDUAL | |
| ), | |
| "save_best_primal_so_far": self.get_parameter( | |
| CUOPT_SAVE_BEST_PRIMAL_SO_FAR | |
| ), | |
| "solution_file": self.get_parameter(CUOPT_SOLUTION_FILE), | |
| "strict_infeasibility": self.get_parameter( | |
| CUOPT_STRICT_INFEASIBILITY | |
| ), | |
| "user_problem_file": self.get_parameter(CUOPT_USER_PROBLEM_FILE), | |
| } | |
| solver_config = {} | |
| solver_config["tolerances"] = {} | |
| for param in solver_params: | |
| if param.endswith("tolerance"): | |
| solver_config["tolerances"][param] = self.get_parameter(param) | |
| else: | |
| param_value = self.get_parameter(param) | |
| if param_value == float("inf"): | |
| param_value = None | |
| solver_config[param] = param_value | |
| def toDict(self): | |
| solver_config = {} | |
| solver_config["tolerances"] = {} | |
| for param in solver_params: | |
| if param.endswith("tolerance"): | |
| solver_config["tolerances"][param] = self.get_parameter(param) | |
| else: | |
| param_value = self.get_parameter(param) | |
| if isinstance(param_value, float) and math.isinf(param_value): | |
| param_value = None | |
| solver_config[param] = param_value |
🤖 Prompt for AI Agents
In `@python/cuopt/cuopt/linear_programming/solver_settings/solver_settings.py`
around lines 282 - 293, The toDict method currently only converts positive
infinity to None (param_value == float("inf")), leaving float("-inf") unchanged;
update to detect any infinity by using math.isinf on values returned from
get_parameter and set param_value to None for both +inf and -inf (ensure to
import math), and guard the check so non-numeric parameter values don't
raise—apply this change inside the loop over solver_params where solver_config
and param_value are handled in toDict.
Iroy30
added
non-breaking
chris-maes
reviewed
Feb 9, 2026
chris-maes
reviewed
Feb 9, 2026
chris-maes
reviewed
Feb 9, 2026
chris-maes
reviewed
Feb 9, 2026
chris-maes
reviewed
Feb 9, 2026
chris-maes
reviewed
Feb 9, 2026
python/cuopt/cuopt/linear_programming/solver_settings/solver_settings.py
Outdated
Show resolved
Hide resolved
chris-maes
reviewed
Feb 9, 2026
python/cuopt/cuopt/linear_programming/solver_settings/solver_settings.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cpp/src/math_optimization/solver_settings.cu`:
- Around line 452-469: The get_parameter_names() method returns duplicate names
because entries from int_parameters, float_parameters, bool_parameters and
string_parameters can overlap; update
solver_settings_t<i_t,f_t>::get_parameter_names() to collect names into a
std::set (or unordered_set) while iterating those vectors (int_parameters,
float_parameters, bool_parameters, string_parameters) and then return a
deduplicated std::vector<std::string> built from that set so callers receive
unique parameter names.
chris-maes
reviewed
Feb 9, 2026
python/cuopt/cuopt/linear_programming/solver_settings/solver_settings.py
Show resolved
Hide resolved
chris-maes
reviewed
Feb 9, 2026
chris-maes
reviewed
Feb 9, 2026
chris-maes
reviewed
Feb 9, 2026
chris-maes
reviewed
Feb 9, 2026
chris-maes
reviewed
Feb 9, 2026
chris-maes
reviewed
Feb 9, 2026
Iroy30
commented
Feb 9, 2026
8 tasks
Member
Author
|
/ok to test a756c8a |
Member
Author
|
/ok to test 56ed4c4 |
Member
Author
|
/ok to test a9b94f6 |
chris-maes
reviewed
Feb 9, 2026
…on_26.02' into release/26.02
Member
Author
|
/ok to test 96c1109 |
rgsl888prabhu
requested changes
Feb 9, 2026
Collaborator
rgsl888prabhu
left a comment
rgsl888prabhu
left a comment
There was a problem hiding this comment.
Are we using these in any docs which might break it ?
python/cuopt/cuopt/linear_programming/solver_settings/solver_settings.py
Show resolved
Hide resolved
Member
Author
|
/ok to test 5d8ebaf |
Member
Author
No docs will not break, user usage is exactly as before. |
chris-maes
approved these changes
Feb 9, 2026
Contributor
chris-maes
left a comment
chris-maes
left a comment
There was a problem hiding this comment.
LGTM. Thanks @Iroy30 . Glad I'll never have to add another parameter name in to the cython file after this PR merges :)
rgsl888prabhu
approved these changes
Feb 9, 2026
Member
Author
|
/merge |
rapids-bot
bot
merged commit ce696eb
into
NVIDIA:release/26.02
196 of 197 checks passed
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.
Description
Issue
Checklist
Summary by CodeRabbit
New Features
Breaking Changes
Improved Interfaces