-
Notifications
You must be signed in to change notification settings - Fork 222
Replace process-wide seed_generator with per-worker RNGs in MIP heuristics #1809
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
381e793
b6b62b9
8295528
c160a86
67b677a
4154f61
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 |
|---|---|---|
|
|
@@ -15,7 +15,6 @@ | |
| #include <mip_heuristics/mip_constants.hpp> | ||
| #include <mip_heuristics/utils.cuh> | ||
| #include <utilities/device_scalar_init.hpp> | ||
| #include <utilities/seed_generator.cuh> | ||
| #include <utilities/timer.hpp> | ||
|
|
||
| #include <raft/linalg/eltwise.cuh> | ||
|
|
@@ -43,8 +42,12 @@ static constexpr int iterations_per_graph = 50; | |
| #endif | ||
|
|
||
| template <typename i_t, typename f_t> | ||
| fj_t<i_t, f_t>::fj_t(mip_solver_context_t<i_t, f_t>& context_, fj_settings_t in_settings) | ||
| : context(context_), | ||
| fj_t<i_t, f_t>::fj_t(mip_solver_context_t<i_t, f_t>& context_, | ||
| fj_settings_t in_settings, | ||
| mip_rng_component_id_t seed_component_id) | ||
| : rng(mip_derive_seed(context_.base_seed, seed_component_id), | ||
| mip_derive_stream(context_.base_seed, seed_component_id)), | ||
|
Comment on lines
+48
to
+49
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. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win Add regression coverage for the new RNG contract. Test that the same base seed and component ID produce a repeatable stream. Test that different component IDs produce separate streams. Cover the per-step seed path and the nearest-rounding fallback. As per coding guidelines, CUDA source changes must add unit tests: " 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| context(context_), | ||
| pb_ptr(context.problem_ptr), | ||
| handle_ptr(const_cast<raft::handle_t*>(pb_ptr->handle_ptr)), | ||
| settings(in_settings), | ||
|
|
@@ -135,12 +138,12 @@ void fj_t<i_t, f_t>::reset_weights(const rmm::cuda_stream_view& climber_stream, | |
| template <typename i_t, typename f_t> | ||
| void fj_t<i_t, f_t>::randomize_weights(const raft::handle_t* handle_ptr) | ||
| { | ||
| std::mt19937 rng(cuopt::seed_generator::get_seed()); | ||
| std::mt19937 host_rng(rng.next_i64()); | ||
| constexpr f_t min_weight = 10.; | ||
| constexpr f_t max_weight = 30.; | ||
| // generate a range of weights between 10. and 30. | ||
| auto h_cstr_vec = | ||
| get_random_uniform_vector<i_t, f_t>(cstr_weights.size(), rng, min_weight, max_weight); | ||
| get_random_uniform_vector<i_t, f_t>(cstr_weights.size(), host_rng, min_weight, max_weight); | ||
| f_t h_max_weight = *std::max_element(h_cstr_vec.begin(), h_cstr_vec.end()); | ||
| max_cstr_weight.set_value_async(h_max_weight, handle_ptr->get_stream()); | ||
| raft::copy(cstr_weights.data(), h_cstr_vec.data(), h_cstr_vec.size(), handle_ptr->get_stream()); | ||
|
|
@@ -672,7 +675,7 @@ void fj_t<i_t, f_t>::run_step_device(const rmm::cuda_stream_view& climber_stream | |
|
|
||
| auto& data = *climbers[climber_idx]; | ||
| auto v = data.view(); | ||
| settings.seed = cuopt::seed_generator::get_seed(); | ||
| settings.seed = rng.next_i64(); | ||
| // ensure an updated copy of the settings is used device-side | ||
| raft::copy(v.settings, &settings, 1, climber_stream); | ||
|
|
||
|
|
@@ -1128,7 +1131,7 @@ i_t fj_t<i_t, f_t>::solve(solution_t<i_t, f_t>& solution) | |
|
|
||
| // if time limit exceeded: round all remaining fractionals if any by nearest rounding. | ||
| if (climbers[0]->fractional_variables.set_size.value(handle_ptr->get_stream()) > 0) { | ||
| solution.round_nearest(); | ||
| solution.round_nearest(rng.next_u64()); | ||
| } | ||
| } | ||
|
|
||
|
|
||
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.
We can change the name from
mip_derive_seedtoderive_seed