-
Notifications
You must be signed in to change notification settings - Fork 112
Objective cut in recombiners #628
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
00ae695
049d07e
b1e7e85
11d853e
5570016
300ad96
6f557c0
b99ed04
d1413c8
60c2620
30375e2
9dc0ace
58b7b2a
28ac0e8
50f3d8e
dca499e
aaa1097
a99d0f4
550635f
f1c977b
de8d2c3
47c62fa
67f5008
2a72dde
1695286
0acb5be
aa78fa6
7fb2ed5
e10e419
fd701f6
b22e3a4
6b26c1b
79e71a1
82a6020
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -495,8 +495,7 @@ void diversity_manager_t<i_t, f_t>::diversity_step(i_t max_iterations_without_im | |
| improved = false; | ||
| while (k-- > 0) { | ||
| if (check_b_b_preemption()) { return; } | ||
| auto new_sol_vector = population.get_external_solutions(); | ||
| recombine_and_ls_with_all(new_sol_vector); | ||
| population.add_external_solutions_to_population(); | ||
| population.adjust_weights_according_to_best_feasible(); | ||
| cuopt_assert(population.test_invariant(), ""); | ||
| if (population.current_size() < 2) { | ||
|
|
@@ -649,7 +648,15 @@ diversity_manager_t<i_t, f_t>::recombine_and_local_search(solution_t<i_t, f_t>& | |
| ls_config_t<i_t, f_t> ls_config; | ||
| ls_config.best_objective_of_parents = best_objective_of_parents; | ||
| ls_config.at_least_one_parent_feasible = at_least_one_parent_feasible; | ||
| success = this->run_local_search(offspring, population.weights, timer, ls_config); | ||
| offspring.swap_problem_pointers(); | ||
| population.weights_with_cuts.cstr_weights.resize(offspring.problem_ptr->n_constraints, | ||
| offspring.handle_ptr->get_stream()); | ||
| raft::copy(population.weights_with_cuts.cstr_weights.data(), | ||
| population.weights.cstr_weights.data(), | ||
| population.weights.cstr_weights.size(), | ||
| offspring.handle_ptr->get_stream()); | ||
|
Comment on lines
+652
to
+657
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am a little worried about maintaining the two weights arrays as well. I fear it would be easy to end up with broken invariants
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it makes sense. We should make it robust as more cuts are coming.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The issue here is that problem is member of the solution and weights is the member of population. I think this might cause more invariants to break. I think the solution to all the problems you pointed out is to use uniform problem for all population and diversity management. Do you think we should tackle this now? |
||
| success = this->run_local_search(offspring, population.weights_with_cuts, timer, ls_config); | ||
| offspring.swap_problem_pointers(); | ||
| if (!success) { | ||
| // add the attempt | ||
| mab_recombiner.add_mab_reward(mab_recombiner.last_chosen_option, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| /* clang-format off */ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2024-2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| * SPDX-FileCopyrightText: Copyright (c) 2024-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
| /* clang-format on */ | ||
|
|
@@ -24,7 +24,6 @@ constexpr double weight_decrease_ratio = 0.9; | |
| constexpr double max_infeasibility_weight = 1e12; | ||
| constexpr double min_infeasibility_weight = 1.; | ||
| constexpr double infeasibility_balance_ratio = 1.1; | ||
| constexpr double halving_skip_ratio = 0.75; | ||
|
|
||
| template <typename i_t, typename f_t> | ||
| population_t<i_t, f_t>::population_t(std::string const& name_, | ||
|
|
@@ -41,8 +40,8 @@ population_t<i_t, f_t>::population_t(std::string const& name_, | |
| max_solutions(max_solutions_), | ||
| infeasibility_importance(infeasibility_weight_), | ||
| weights(0, context.problem_ptr->handle_ptr), | ||
| weights_with_cuts(0, context.problem_ptr->handle_ptr), | ||
| rng(cuopt::seed_generator::get_seed()), | ||
| early_exit_primal_generation(false), | ||
| population_hash_map(*problem_ptr), | ||
| timer(0) | ||
| { | ||
|
|
@@ -64,6 +63,14 @@ i_t get_max_var_threshold(i_t n_vars) | |
| return n_vars - 10; | ||
| } | ||
|
|
||
| template <typename i_t, typename f_t> | ||
| void population_t<i_t, f_t>::apply_problem_ptr_to_all_solutions() | ||
| { | ||
| for (size_t i = 0; i < indices.size(); i++) { | ||
| solutions[indices[i].first].second.problem_with_cuts_ptr = problem_ptr_with_cuts; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we could benefit from adding a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is already a flag for the objective cut. We can use that. |
||
| } | ||
| } | ||
|
|
||
| template <typename i_t, typename f_t> | ||
| void population_t<i_t, f_t>::allocate_solutions() | ||
| { | ||
|
|
@@ -73,6 +80,20 @@ void population_t<i_t, f_t>::allocate_solutions() | |
| } | ||
| } | ||
|
|
||
| template <typename i_t, typename f_t> | ||
| void population_t<i_t, f_t>::set_problem_ptr_with_cuts(problem_t<i_t, f_t>* problem_ptr_with_cuts) | ||
| { | ||
| constexpr f_t ten = 10.; | ||
| this->problem_ptr_with_cuts = problem_ptr_with_cuts; | ||
| weights_with_cuts.cstr_weights.resize(problem_ptr_with_cuts->n_constraints, | ||
| problem_ptr_with_cuts->handle_ptr->get_stream()); | ||
| // fill last element with default | ||
| thrust::uninitialized_fill(problem_ptr_with_cuts->handle_ptr->get_thrust_policy(), | ||
| weights_with_cuts.cstr_weights.begin() + problem_ptr->n_constraints, | ||
| weights_with_cuts.cstr_weights.end(), | ||
| ten); | ||
| } | ||
|
Comment on lines
+83
to
+95
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Search for calls to set_problem_ptr_with_cuts
rg -n -B5 -A10 'set_problem_ptr_with_cuts\(' --type cpp --type cuRepository: NVIDIA/cuopt Length of output: 82 🏁 Script executed: # Look at the population class definition and weight initialization
fd -e cu -e cpp -path "*population*" | head -20Repository: NVIDIA/cuopt Length of output: 224 🏁 Script executed: # Search for weights_with_cuts initialization
rg -n 'weights_with_cuts' --type cpp --type cu -A2 -B2Repository: NVIDIA/cuopt Length of output: 82 🏁 Script executed: # Search for calls to set_problem_ptr_with_cuts using corrected syntax
rg -n -B5 -A10 'set_problem_ptr_with_cuts\('Repository: NVIDIA/cuopt Length of output: 7170 🏁 Script executed: # Find population files
fd 'population' -e cu -e cpp -e h -e hppRepository: NVIDIA/cuopt Length of output: 132 🏁 Script executed: # Search for weights_with_cuts references
rg -n 'weights_with_cuts' -A2 -B2Repository: NVIDIA/cuopt Length of output: 4894 🏁 Script executed: # Let's look at the specific context around the three call sites to understand the flow better
sed -n '540,560p' cpp/src/mip/local_search/local_search.cuRepository: NVIDIA/cuopt Length of output: 1021 🏁 Script executed: # Check the second call site context
sed -n '595,610p' cpp/src/mip/local_search/local_search.cuRepository: NVIDIA/cuopt Length of output: 829 🏁 Script executed: # Check the third call site context
sed -n '645,660p' cpp/src/mip/local_search/local_search.cuRepository: NVIDIA/cuopt Length of output: 836 🏁 Script executed: # Look at the diversity_manager call context
sed -n '645,660p' cpp/src/mip/diversity/diversity_manager.cuRepository: NVIDIA/cuopt Length of output: 962 🏁 Script executed: # Check the constructor/initialization of weights_with_cuts
sed -n '35,50p' cpp/src/mip/diversity/population.cuRepository: NVIDIA/cuopt Length of output: 539 🏁 Script executed: # Check the apply_problem_ptr_to_all_solutions method to understand state changes
rg -n 'apply_problem_ptr_to_all_solutions' -A 20Repository: NVIDIA/cuopt Length of output: 10454 🏁 Script executed: # Check the update_weights method and other weight-related methods
sed -n '180,220p' cpp/src/mip/diversity/population.cuRepository: NVIDIA/cuopt Length of output: 1669 🏁 Script executed: # Check if weights_with_cuts is used before being properly initialized at call site 3
rg -n 'weights_with_cuts' cpp/src/mip/local_search/local_search.cu -B3 -A3 | head -80Repository: NVIDIA/cuopt Length of output: 826 🏁 Script executed: # Verify what happens between fj.copy_weights and set_problem_ptr_with_cuts at line 648-650
sed -n '645,655p' cpp/src/mip/local_search/local_search.cuRepository: NVIDIA/cuopt Length of output: 661 🏁 Script executed: # Let's check if weights_with_cuts is actually used right after set_problem_ptr_with_cuts at call site 3
sed -n '648,700p' cpp/src/mip/local_search/local_search.cu | head -60Repository: NVIDIA/cuopt Length of output: 2370 🏁 Script executed: # Check if there's a pattern where weights are supposed to be copied elsewhere
# Look for update_weights or other weight-related operations
sed -n '117,140p' cpp/src/mip/diversity/population.cuRepository: NVIDIA/cuopt Length of output: 1255 🏁 Script executed: # Check if weights_with_cuts is used after call site 3 and understand the intent
# Look for uses of weights_with_cuts in the local_search context
rg -n 'weights_with_cuts' cpp/src/mip/local_search/local_search.cu -B2 -A2Repository: NVIDIA/cuopt Length of output: 626 🏁 Script executed: # Check what copy_weights does - it should help understand what should happen
rg -n 'copy_weights' -A 15 cpp/src/mip/diversity/Repository: NVIDIA/cuopt Length of output: 2107 🏁 Script executed: # Let's understand the code flow at call site 3 more clearly
# Check what happens right before and after set_problem_ptr_with_cuts at line 650
sed -n '643,655p' cpp/src/mip/local_search/local_search.cuRepository: NVIDIA/cuopt Length of output: 771 🏁 Script executed: # Now let's verify: does weights_with_cuts actually need the old constraint weights?
# Check how it's used after being set
rg -n 'weights_with_cuts' cpp/src/mip/diversity/ cpp/src/mip/local_search/ -B1 -A1 | grep -E '(run_local_search|update_weights|diversity_step)'Repository: NVIDIA/cuopt Length of output: 197 🏁 Script executed: # Check the third call site more carefully - look for any subsequent copy operations
sed -n '645,670p' cpp/src/mip/local_search/local_search.cuRepository: NVIDIA/cuopt Length of output: 1338 🏁 Script executed: # Let's check what happens in the loop after set_problem_ptr_with_cuts at call site 3
# Specifically, does weights_with_cuts get used before being properly initialized?
sed -n '630,670p' cpp/src/mip/local_search/local_search.cu | head -40Repository: NVIDIA/cuopt Length of output: 2151 🏁 Script executed: # Let's trace the full context of whether weights_with_cuts is initialized at call site 3
# Check if there's any copy between fj.copy_weights and the loop that uses weights_with_cuts
sed -n '643,680p' cpp/src/mip/local_search/local_search.cuRepository: NVIDIA/cuopt Length of output: 1837 🏁 Script executed: # Search for where weights_with_cuts is actually used in this context
rg -n 'weights_with_cuts' cpp/src/mip/local_search/ -C 5Repository: NVIDIA/cuopt Length of output: 2062 🏁 Script executed: # Let's verify the issue by checking what resize does to the existing data
# and understand the memory state more clearly
rg -n 'resize\(' cpp/src/mip/diversity/population.cu -B2 -A2Repository: NVIDIA/cuopt Length of output: 1043 🏁 Script executed: # Check if there's any documentation or comments about weights_with_cuts initialization
rg -n 'weights_with_cuts' cpp/src/mip/diversity/population.cuh -B1 -A1Repository: NVIDIA/cuopt Length of output: 184 🏁 Script executed: # Confirm the exact problem: at call site 3, is weights_with_cuts actually used?
# Check if it's used in the subsequent code path
sed -n '650,680p' cpp/src/mip/local_search/local_search.cuRepository: NVIDIA/cuopt Length of output: 1398 🏁 Script executed: # Let's check if weights_with_cuts is used anywhere in the fp loop after call site 3
rg -n 'weights_with_cuts' cpp/src/mip/local_search/local_search.cu -B5 -A5Repository: NVIDIA/cuopt Length of output: 1160 🏁 Script executed: # Check what resize_to_new_problem does and if it uses weights_with_cuts
rg -n 'resize_to_new_problem' cpp/src/mip/local_search/local_search.cu -A 20 | head -50Repository: NVIDIA/cuopt Length of output: 2615 🏁 Script executed: # Let's check the weight_t class to understand what resize does
fd -e h -e hpp | xargs grep -l 'class weight_t' | head -3Repository: NVIDIA/cuopt Length of output: 38 🏁 Script executed: # Let's check the weight_t class definition to understand the resize behavior
fd weight_t -e h -e hpp -e cu | head -5Repository: NVIDIA/cuopt Length of output: 38 🏁 Script executed: # Search for weight_t definition
rg -n 'class weight_t\|struct weight_t' -A 10Repository: NVIDIA/cuopt Length of output: 38 🏁 Script executed: # Search for weight_t definition more broadly
rg 'weight_t' -A 5 cpp/src/mip/diversity/ | grep -E '(struct|class|resize)' | head -20Repository: NVIDIA/cuopt Length of output: 175 🏁 Script executed: # Let's check the actual behavior: when resize is called on an existing vector
# The key question is whether the existing data is preserved or overwritten
# Let's look at the cstr_weights member
rg -n 'cstr_weights' cpp/src/mip/diversity/ -B2 -A2 | head -50Repository: NVIDIA/cuopt Length of output: 4275 🏁 Script executed: # Critical check: at call site 3 (line 650), after resize, what are the first problem_ptr->n_constraints elements?
# They should either be: (1) preserved from before, or (2) uninitialized
# Let's verify by checking if there's any copy_weights or similar before the loop
sed -n '630,660p' cpp/src/mip/local_search/local_search.cuRepository: NVIDIA/cuopt Length of output: 1704 🏁 Script executed: # Now let's check the weight_t struct to understand the resize behavior
cat cpp/src/mip/diversity/weights.cuh | head -100Repository: NVIDIA/cuopt Length of output: 1069 🏁 Script executed: # Check if weights_with_cuts is used after call site 3 in any way
rg -n 'population_ptr->weights_with_cuts\|population\.weights_with_cuts' cpp/src/mip/local_search/ -B2 -A2Repository: NVIDIA/cuopt Length of output: 38 🏁 Script executed: # Let's check what rmm::device_uvector::resize does
# And let's verify whether weights_with_cuts is actually used after call site 3
rg -n 'reset_alpha_and_save_solution' cpp/src/mip/local_search/local_search.cu -A 5Repository: NVIDIA/cuopt Length of output: 1537 🏁 Script executed: # Look for the actual issue: at call site 3, after set_problem_ptr_with_cuts,
# is there any guarantee that the first problem_ptr->n_constraints elements are initialized?
# Let's check the state of weights_with_cuts before each call site
sed -n '540,565p' cpp/src/mip/local_search/local_search.cuRepository: NVIDIA/cuopt Length of output: 1216 🏁 Script executed: # Final verification: Check the state at call site 3 - is weights_with_cuts used later in the loop?
sed -n '650,700p' cpp/src/mip/local_search/local_search.cu | grep -i weightRepository: NVIDIA/cuopt Length of output: 38 🏁 Script executed: # Check if there's supposed to be a copy similar to call sites 1 & 2
# Look at the pattern in call sites 1 and 2 right after set_problem_ptr_with_cuts
sed -n '545,562p' cpp/src/mip/local_search/local_search.cuRepository: NVIDIA/cuopt Length of output: 974 Add raft::copy call after set_problem_ptr_with_cuts at line 650 to initialize constraint weights. At line 650, raft::copy(population_ptr->weights_with_cuts.cstr_weights.data(),
fj.cstr_weights.data(),
fj.cstr_weights.size(),
solution.handle_ptr->get_stream());after line 650 to match the pattern used at the other call sites. 🤖 Prompt for AI Agents |
||
|
|
||
| template <typename i_t, typename f_t> | ||
| void population_t<i_t, f_t>::initialize_population() | ||
| { | ||
|
|
@@ -87,6 +108,12 @@ void population_t<i_t, f_t>::initialize_population() | |
| weights.cstr_weights.begin(), | ||
| weights.cstr_weights.end(), | ||
| ten); | ||
| weights_with_cuts.cstr_weights.resize(problem_ptr->n_constraints, | ||
| problem_ptr->handle_ptr->get_stream()); | ||
| thrust::uninitialized_fill(problem_ptr->handle_ptr->get_thrust_policy(), | ||
| weights_with_cuts.cstr_weights.begin(), | ||
| weights_with_cuts.cstr_weights.end(), | ||
| ten); | ||
| } | ||
|
|
||
| template <typename i_t, typename f_t> | ||
|
|
@@ -109,12 +136,11 @@ std::pair<solution_t<i_t, f_t>, solution_t<i_t, f_t>> population_t<i_t, f_t>::ge | |
| auto second_solution = solutions[indices[j].first].second; | ||
| // if best feasible and best are the same, take the second index instead of best | ||
| if (i == 0 && j == 1) { | ||
| bool same = | ||
| check_integer_equal_on_indices(first_solution.problem_ptr->integer_indices, | ||
| first_solution.assignment, | ||
| second_solution.assignment, | ||
| first_solution.problem_ptr->tolerances.integrality_tolerance, | ||
| first_solution.handle_ptr); | ||
| bool same = check_integer_equal_on_indices(problem_ptr->integer_indices, | ||
| first_solution.assignment, | ||
| second_solution.assignment, | ||
| problem_ptr->tolerances.integrality_tolerance, | ||
| first_solution.handle_ptr); | ||
| if (same) { | ||
| auto new_sol = solutions[indices[2].first].second; | ||
| second_solution = std::move(new_sol); | ||
|
|
@@ -172,7 +198,6 @@ void population_t<i_t, f_t>::add_external_solution(const std::vector<f_t>& solut | |
| CUOPT_LOG_DEBUG("Found new best solution %g in external queue", | ||
| problem_ptr->get_user_obj_from_solver_obj(objective)); | ||
| } | ||
| if (external_solution_queue.size() >= 5) { early_exit_primal_generation = true; } | ||
| solutions_in_external_queue_ = true; | ||
| } | ||
|
|
||
|
|
@@ -184,6 +209,7 @@ void population_t<i_t, f_t>::add_external_solutions_to_population() | |
|
|
||
| auto new_sol_vector = get_external_solutions(); | ||
| add_solutions_from_vec(std::move(new_sol_vector)); | ||
| apply_problem_ptr_to_all_solutions(); | ||
| } | ||
|
|
||
| // normally we would need a lock here but these are boolean types and race conditions are not | ||
|
|
@@ -192,7 +218,6 @@ template <typename i_t, typename f_t> | |
| void population_t<i_t, f_t>::preempt_heuristic_solver() | ||
| { | ||
| context.preempt_heuristic_solver_ = true; | ||
| early_exit_primal_generation = true; | ||
| } | ||
|
|
||
| template <typename i_t, typename f_t> | ||
|
|
@@ -668,42 +693,6 @@ std::vector<solution_t<i_t, f_t>> population_t<i_t, f_t>::population_to_vector() | |
| return sol_vec; | ||
| } | ||
|
|
||
| template <typename i_t, typename f_t> | ||
| void population_t<i_t, f_t>::halve_the_population() | ||
| { | ||
| raft::common::nvtx::range fun_scope("halve_the_population"); | ||
| // try 3/4 here | ||
| if (current_size() <= (max_solutions * halving_skip_ratio)) { return; } | ||
| CUOPT_LOG_DEBUG("Halving the population, current size: %lu", current_size()); | ||
| // put population into a vector | ||
| auto sol_vec = population_to_vector(); | ||
| i_t counter = 0; | ||
| constexpr i_t max_adjustments = 4; | ||
| size_t max_var_threshold = get_max_var_threshold(problem_ptr->n_integer_vars); | ||
|
|
||
| std::lock_guard<std::recursive_mutex> lock(write_mutex); | ||
| while (current_size() > max_solutions / 2) { | ||
| clear_except_best_feasible(); | ||
| var_threshold = std::max(var_threshold * 0.97, 0.5 * problem_ptr->n_integer_vars); | ||
| for (auto& sol : sol_vec) { | ||
| add_solution(solution_t<i_t, f_t>(sol)); | ||
| } | ||
| if (counter++ > max_adjustments) break; | ||
| } | ||
| counter = 0; | ||
| // if we removed too many decrease the diversity a little | ||
| while (current_size() < max_solutions / 4) { | ||
| clear_except_best_feasible(); | ||
| var_threshold = std::min( | ||
| max_var_threshold, | ||
| std::min((size_t)(var_threshold * 1.02), (size_t)(0.995 * problem_ptr->n_integer_vars))); | ||
| for (auto& sol : sol_vec) { | ||
| add_solution(solution_t<i_t, f_t>(sol)); | ||
| } | ||
| if (counter++ > max_adjustments) break; | ||
| } | ||
| } | ||
|
|
||
| template <typename i_t, typename f_t> | ||
| size_t population_t<i_t, f_t>::find_free_solution_index() | ||
| { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential division by zero if
n_gpusis 0.While
n_gpusdefaults to 1, there's no validation preventing a user from passing--n-gpus 0, which would cause undefined behavior here. Consider adding an early validation check.🛡️ Suggested validation (add after line 367)
🤖 Prompt for AI Agents