Conversation
nguidotti
added
non-breaking
CI Test Summary⏭️ All 5 test job(s) skipped. |
| return; | ||
| } | ||
|
|
||
| if (halt != nullptr && halt->load(std::memory_order_acquire)) { return; } |
There was a problem hiding this comment.
Use submip_settings.concurrent_halt here for consistency
| f_t fixrate, | ||
| i_t simplex_iter_used) | ||
| i_t simplex_iter_used, | ||
| std::atomic<int>* halt) |
There was a problem hiding this comment.
It's a bit strange to be passing the halt pointers around. They are meant to be contained within the settings. It might be better to create the settings with the halt and pass the settings in here.
| const std::vector<f_t>& current_incumbent, | ||
| const std::vector<variable_type_t>& var_types) | ||
| const std::vector<variable_type_t>& var_types, | ||
| std::atomic<int>* halt) |
There was a problem hiding this comment.
Same here. Could you just pass in the settings with the halt set?
|
|
||
| while (solver_status_ == mip_status_t::UNSET && is_running_ && !worker->halt) { | ||
| while (solver_status_ == mip_status_t::UNSET && is_running_ && | ||
| !(halt && halt->load(std::memory_order::acquire))) { |
There was a problem hiding this comment.
Could we use submip_settings.concurrent_halt here instead of a bare halt.
| f_t root_relax_objective, | ||
| i_t& cut_pool_size, | ||
| [[maybe_unused]] const std::vector<f_t>& saved_solution) -> cut_pass_result_t | ||
| [[maybe_unused]] const std::vector<f_t>& saved_solution) -> cut_pass_action_t |
There was a problem hiding this comment.
Good to see the return type simiplified
There was a problem hiding this comment.
Would you mind replacing the auto with cut_pass_action_t and removing the -> cut_pass_action_t here?
| f_t root_relax_elapsed_time = toc(root_relax_start_time); | ||
| exploration_stats_.total_lp_solve_time = root_relax_elapsed_time; | ||
|
|
||
| scope_guard cliques_scope([&]() { |
There was a problem hiding this comment.
I'm not a fan of scope guards. These make it difficult to understand what is happening in the code. Since code is executing that may be defined far away from the actual return statement.
There was a problem hiding this comment.
I understand that it is helpful to avoid having to have do this clean up at every return though. Is it possible to put this in the cliques destructor instead maybe?
There was a problem hiding this comment.
If not, could you add a comment here explaining the need for the scope guard; so a reader is aware.
| csr_matrix_t<i_t, f_t> Arow_; | ||
| std::vector<f_t> root_solution_; | ||
| std::vector<f_t> root_edge_norm_; | ||
| std::atomic<int> halt_; |
There was a problem hiding this comment.
Why do we need a bare halt here? Can we store the halt in settings?
There was a problem hiding this comment.
We need to store the actual object somewhere, right? The concurrent_halt points to this object.
Signed-off-by: Nicolas L. Guidotti <nguidotti@nvidia.com>
Signed-off-by: Nicolas L. Guidotti <nguidotti@nvidia.com>
Signed-off-by: Nicolas L. Guidotti <nguidotti@nvidia.com>
Signed-off-by: Nicolas L. Guidotti <nguidotti@nvidia.com>
2 participants
With this PR, the B&B code can now be stopped via the
concurrent_haltflag in thesimplex_solver_settings. This also fixes the improper handle of theconcurrent_haltflag during the cut passes and replaces the sub-MIP halt mechanism with the newer version.Checklist