-
Notifications
You must be signed in to change notification settings - Fork 239
Include objective gap in QP termination criteria #1733
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
Changes from all commits
e1b24f2
269cbd9
29bce4f
ed622ed
1fedd5a
3936874
d9513a7
de0bd77
6742aae
51b7dc8
cd9e70c
02a258e
168a1b4
711f1f8
6b0f332
d397bbd
89a6f8d
79a2898
f8668f6
3cd2e17
1ede03d
8dd5d19
e3e1e0f
47e106d
9625c54
97065f7
04e8c46
4a72eb3
3c17339
f899ef4
0837cad
e3d42bb
74be02a
1ffe501
6f2eb49
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 |
|---|---|---|
|
|
@@ -2203,6 +2203,8 @@ class iteration_data_t { | |
| f_t dual_residual_norm_save; | ||
| f_t complementarity_residual_norm_save; | ||
|
|
||
| bool use_high_accuracy_ir = false; | ||
|
|
||
| dense_vector_t<i_t, f_t> diag; | ||
| pinned_dense_vector_t<i_t, f_t> inv_diag; | ||
| dense_vector_t<i_t, f_t> inv_sqrt_diag; | ||
|
|
@@ -2581,7 +2583,8 @@ int barrier_solver_t<i_t, f_t>::initial_point(iteration_data_t<i_t, f_t>& data) | |
| } op(data); | ||
|
|
||
| if (settings.barrier_iterative_refinement) { | ||
| const f_t ir_tol = data.has_sparse_cones() ? f_t(1e-12) : f_t(1e-8); | ||
| const f_t ir_tol = | ||
| (data.has_sparse_cones() || data.use_high_accuracy_ir) ? f_t(1e-12) : f_t(1e-8); | ||
| iterative_refinement<i_t, f_t, op_t>(op, rhs, soln, ir_tol); | ||
| } | ||
|
|
||
|
|
@@ -3175,7 +3178,8 @@ i_t barrier_solver_t<i_t, f_t>::gpu_compute_search_direction(iteration_data_t<i_ | |
| } op(data); | ||
| if (settings.barrier_iterative_refinement) { | ||
| raft::common::nvtx::range fun_scope("Barrier: iterative_refinement"); | ||
| const f_t ir_tol = data.has_sparse_cones() ? f_t(1e-12) : f_t(1e-8); | ||
| const f_t ir_tol = | ||
| (data.has_sparse_cones() || data.use_high_accuracy_ir) ? f_t(1e-12) : f_t(1e-8); | ||
| const f_t solve_err = iterative_refinement<i_t, f_t, op_t>( | ||
| op, data.d_augmented_rhs_, data.d_augmented_soln_, ir_tol); | ||
| if (solve_err > 1e-1) { | ||
|
|
@@ -3263,8 +3267,9 @@ i_t barrier_solver_t<i_t, f_t>::gpu_compute_search_direction(iteration_data_t<i_ | |
| data_.gpu_solve_adat(b, x); | ||
| } | ||
| } adat_op(data); | ||
| const f_t ir_tol = 1e-8; | ||
| const f_t adat_solve_err = | ||
| iterative_refinement<i_t, f_t, adat_op_t>(adat_op, data.d_h_, data.d_dy_); | ||
| iterative_refinement<i_t, f_t, adat_op_t>(adat_op, data.d_h_, data.d_dy_, ir_tol); | ||
| if (adat_solve_err > 1e-1) { | ||
| settings.log.debug("||ADAT*dy - h|| %e after IR\n", adat_solve_err); | ||
| } | ||
|
|
@@ -4266,16 +4271,20 @@ lp_status_t barrier_solver_t<i_t, f_t>::check_for_suboptimal_solution( | |
| f_t& primal_residual_norm, | ||
| f_t& dual_residual_norm, | ||
| f_t& complementarity_residual_norm, | ||
| f_t& objective_gap, | ||
| f_t& relative_primal_residual, | ||
| f_t& relative_dual_residual, | ||
| f_t& relative_complementarity_residual, | ||
| f_t& relative_objective_gap, | ||
| lp_solution_t<i_t, f_t>& solution) | ||
| { | ||
| raft::common::nvtx::range fun_scope("Barrier: check_for_suboptimal_solution"); | ||
| bool small_gap = (!data.has_cones() && data.Q.n == 0) || | ||
| relative_objective_gap < settings.barrier_relaxed_relative_objective_gap_tol; | ||
| if (relative_primal_residual < settings.barrier_relaxed_feasibility_tol && | ||
| relative_dual_residual < settings.barrier_relaxed_optimality_tol && | ||
| relative_complementarity_residual < settings.barrier_relaxed_complementarity_tol && | ||
|
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. The purpose of |
||
| primal_objective == primal_objective) { | ||
| small_gap && primal_objective == primal_objective) { | ||
| raft::copy(data.x.data(), data.d_x_.data(), data.d_x_.size(), stream_view_); | ||
| raft::copy(data.y.data(), data.d_y_.data(), data.d_y_.size(), stream_view_); | ||
| raft::copy(data.z.data(), data.d_z_.data(), data.d_z_.size(), stream_view_); | ||
|
|
@@ -4300,24 +4309,39 @@ lp_status_t barrier_solver_t<i_t, f_t>::check_for_suboptimal_solution( | |
| settings.log.printf("Complementarity gap (abs/rel): %8.2e/%8.2e\n", | ||
| complementarity_residual_norm, | ||
| relative_complementarity_residual); | ||
| settings.log.printf( | ||
| "Objective gap (abs/rel): %8.2e/%8.2e\n", objective_gap, relative_objective_gap); | ||
| settings.log.printf("\n"); | ||
| return lp_status_t::OPTIMAL; // TODO: Barrier should probably have a separate suboptimal | ||
| // status | ||
| } | ||
|
|
||
| f_t primal_objective_save = data.c.inner_product(data.x_save); | ||
| f_t dual_objective_save = | ||
| data.b.inner_product(data.y_save) - data.restrict_u_.inner_product(data.v_save); | ||
| if (data.Q.n > 0) { | ||
| dense_vector_t<i_t, f_t> Qx_save(data.Q.n); | ||
| dense_vector_t<i_t, f_t> x_save_host(data.Q.n); | ||
| std::copy(data.x_save.begin(), data.x_save.begin() + data.Q.n, x_save_host.begin()); | ||
| matrix_vector_multiply(data.Q, 1.0, x_save_host, 0.0, Qx_save); | ||
| f_t quad_objective = 0.5 * x_save_host.inner_product(Qx_save); | ||
| primal_objective_save += quad_objective; | ||
| dual_objective_save -= quad_objective; | ||
| } | ||
|
|
||
| f_t objective_gap_save = std::abs(primal_objective_save - dual_objective_save); | ||
| f_t user_primal_objective_save = compute_user_objective(lp, primal_objective_save); | ||
| f_t relative_objective_gap_save = | ||
|
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. Only primal info is used in computing
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. How would you use that info in the denominator? Take the min over the primal and dual objectives?
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 can take the max over the absolute values of primal and dual, and then min operation over solver objective and user objective.
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. Why max though? min is much more conservative
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. Yeah, min is stricter. The point is to include both primal and dual info for computation at this line. |
||
| objective_gap_save / | ||
| (1.0 + std::min(std::abs(user_primal_objective_save), std::abs(primal_objective_save))); | ||
| bool small_gap_save = | ||
| (!data.has_cones() && data.Q.n == 0) || | ||
| relative_objective_gap_save < settings.barrier_relaxed_relative_objective_gap_tol; | ||
|
|
||
| if (data.relative_primal_residual_save < settings.barrier_relaxed_feasibility_tol && | ||
| data.relative_dual_residual_save < settings.barrier_relaxed_optimality_tol && | ||
| data.relative_complementarity_residual_save < settings.barrier_relaxed_complementarity_tol) { | ||
| data.relative_complementarity_residual_save < settings.barrier_relaxed_complementarity_tol && | ||
| small_gap_save) { | ||
| settings.log.printf("Restoring previous solution\n"); | ||
| data.restore_saved_iterate(); | ||
| data.to_solution(lp, | ||
|
|
@@ -4340,14 +4364,19 @@ lp_status_t barrier_solver_t<i_t, f_t>::check_for_suboptimal_solution( | |
| settings.log.printf("Complementarity gap (abs/rel): %8.2e/%8.2e\n", | ||
| data.complementarity_residual_norm_save, | ||
| data.relative_complementarity_residual_save); | ||
| settings.log.printf("Objective gap (abs/rel): %8.2e/%8.2e\n", | ||
| objective_gap_save, | ||
| relative_objective_gap_save); | ||
| settings.log.printf("\n"); | ||
| return lp_status_t::OPTIMAL; // TODO: Barrier should probably have a separate suboptimal | ||
| // status | ||
| } else { | ||
| settings.log.printf("Primal residual %.2e dual residual %.2e complementarity residual %.2e\n", | ||
| relative_primal_residual, | ||
| relative_dual_residual, | ||
| relative_complementarity_residual); | ||
| settings.log.printf( | ||
| "Primal residual %.2e dual residual %.2e complementarity residual %.2e objective gap %.2e\n", | ||
| relative_primal_residual, | ||
| relative_dual_residual, | ||
| relative_complementarity_residual, | ||
| relative_objective_gap); | ||
| } | ||
| settings.log.printf("Search direction computation failed\n"); | ||
| return lp_status_t::NUMERICAL_ISSUES; | ||
|
|
@@ -4449,18 +4478,18 @@ lp_status_t barrier_solver_t<i_t, f_t>::barrier_advanced_solve(f_t start_time, | |
| mu, | ||
| primal_objective, | ||
| dual_objective); | ||
| f_t user_primal_objective = compute_user_objective(lp, primal_objective); | ||
| f_t user_dual_objective = compute_user_objective(lp, dual_objective); | ||
|
|
||
| f_t relative_primal_residual = primal_residual_norm / (1.0 + norm_b); | ||
| f_t relative_dual_residual = dual_residual_norm / (1.0 + norm_c); | ||
| f_t relative_complementarity_residual = | ||
| complementarity_residual_norm / | ||
| (1.0 + std::min(std::abs(compute_user_objective(lp, primal_objective)), | ||
| std::abs(primal_objective))); | ||
| (1.0 + std::min(std::abs(user_primal_objective), std::abs(primal_objective))); | ||
|
|
||
| f_t objective_gap_abs = std::abs(primal_objective - dual_objective); | ||
| f_t objective_gap_rel = | ||
| objective_gap_abs / | ||
| std::max(f_t(1), std::min(std::abs(primal_objective), std::abs(dual_objective))); | ||
| f_t objective_gap, relative_objective_gap; | ||
| compute_objective_gap( | ||
| lp, primal_objective, dual_objective, objective_gap, relative_objective_gap); | ||
|
|
||
| data.w_save = data.w; | ||
| data.x_save = data.x; | ||
|
|
@@ -4477,16 +4506,19 @@ lp_status_t barrier_solver_t<i_t, f_t>::barrier_advanced_solve(f_t start_time, | |
| float64_t elapsed_time = toc(start_time); | ||
| settings.log.printf("%3d %+.12e %+.12e %.2e %.2e %.2e %.1f\n", | ||
| iter, | ||
| compute_user_objective(lp, primal_objective), | ||
| compute_user_objective(lp, dual_objective), | ||
| user_primal_objective, | ||
| user_dual_objective, | ||
| relative_primal_residual, | ||
| relative_dual_residual, | ||
| relative_complementarity_residual, | ||
| elapsed_time); | ||
|
|
||
| bool converged = primal_residual_norm < settings.barrier_relative_feasibility_tol && | ||
| dual_residual_norm < settings.barrier_relative_optimality_tol && | ||
| complementarity_residual_norm < settings.barrier_relative_complementarity_tol; | ||
| bool small_gap = (!data.has_cones() && data.Q.n == 0) || | ||
|
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.
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. Let's test if we can do this for all problems. Hopefully, we can.
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. Not yet, we have regressions on LP barrier |
||
| relative_objective_gap < settings.barrier_relaxed_relative_objective_gap_tol; | ||
| bool converged = | ||
| primal_residual_norm < settings.barrier_relative_feasibility_tol && | ||
| dual_residual_norm < settings.barrier_relative_optimality_tol && | ||
| complementarity_residual_norm < settings.barrier_relative_complementarity_tol && small_gap; | ||
|
|
||
| const i_t iteration_limit = settings.iteration_limit; | ||
|
|
||
|
|
@@ -4535,9 +4567,11 @@ lp_status_t barrier_solver_t<i_t, f_t>::barrier_advanced_solve(f_t start_time, | |
| primal_residual_norm, | ||
| dual_residual_norm, | ||
| complementarity_residual_norm, | ||
| objective_gap, | ||
| relative_primal_residual, | ||
| relative_dual_residual, | ||
| relative_complementarity_residual, | ||
| relative_objective_gap, | ||
| solution); | ||
| } | ||
| if (toc(start_time) > settings.time_limit) { | ||
|
|
@@ -4575,9 +4609,11 @@ lp_status_t barrier_solver_t<i_t, f_t>::barrier_advanced_solve(f_t start_time, | |
| primal_residual_norm, | ||
| dual_residual_norm, | ||
| complementarity_residual_norm, | ||
| objective_gap, | ||
| relative_primal_residual, | ||
| relative_dual_residual, | ||
| relative_complementarity_residual, | ||
| relative_objective_gap, | ||
| solution); | ||
| } | ||
| data.has_factorization = false; | ||
|
|
@@ -4605,17 +4641,20 @@ lp_status_t barrier_solver_t<i_t, f_t>::barrier_advanced_solve(f_t start_time, | |
| primal_objective, | ||
| dual_objective); | ||
|
|
||
| relative_primal_residual = primal_residual_norm / (1.0 + norm_b); | ||
| relative_dual_residual = dual_residual_norm / (1.0 + norm_c); | ||
| f_t user_primal_objective = compute_user_objective(lp, primal_objective); | ||
| relative_primal_residual = primal_residual_norm / (1.0 + norm_b); | ||
| relative_dual_residual = dual_residual_norm / (1.0 + norm_c); | ||
| relative_complementarity_residual = | ||
| complementarity_residual_norm / | ||
| (1.0 + std::min(std::abs(compute_user_objective(lp, primal_objective)), | ||
| std::abs(primal_objective))); | ||
| (1.0 + std::min(std::abs(user_primal_objective), std::abs(primal_objective))); | ||
|
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. Same issue for missing dual info. |
||
|
|
||
| objective_gap_abs = std::abs(primal_objective - dual_objective); | ||
| objective_gap_rel = | ||
| objective_gap_abs / | ||
| std::max(f_t(1), std::min(std::abs(primal_objective), std::abs(dual_objective))); | ||
| compute_objective_gap( | ||
| lp, primal_objective, dual_objective, objective_gap, relative_objective_gap); | ||
|
|
||
| if (data.has_cones() && !data.use_high_accuracy_ir && relative_primal_residual < 1e-6 && | ||
| relative_dual_residual < 1e-6 && relative_complementarity_residual < 1e-6) { | ||
| data.use_high_accuracy_ir = true; | ||
| } | ||
|
|
||
| if (relative_primal_residual < settings.barrier_relaxed_feasibility_tol && | ||
| relative_dual_residual < settings.barrier_relaxed_optimality_tol && | ||
|
|
@@ -4663,9 +4702,11 @@ lp_status_t barrier_solver_t<i_t, f_t>::barrier_advanced_solve(f_t start_time, | |
| primal_residual_norm, | ||
| dual_residual_norm, | ||
| complementarity_residual_norm, | ||
| objective_gap, | ||
| relative_primal_residual, | ||
| relative_dual_residual, | ||
| relative_complementarity_residual, | ||
| relative_objective_gap, | ||
| solution); | ||
| } | ||
|
|
||
|
|
@@ -4683,7 +4724,8 @@ lp_status_t barrier_solver_t<i_t, f_t>::barrier_advanced_solve(f_t start_time, | |
| bool small_gap = | ||
| relative_complementarity_residual < settings.barrier_relative_complementarity_tol; | ||
| bool small_objective_gap = | ||
| !data.has_cones() || objective_gap_rel < settings.barrier_relaxed_complementarity_tol; | ||
| (!data.has_cones() && data.Q.n == 0) || | ||
|
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. Same concern for |
||
| relative_objective_gap < settings.barrier_relative_objective_gap_tol; | ||
|
|
||
| converged = primal_feasible && dual_feasible && small_gap && small_objective_gap; | ||
|
|
||
|
|
@@ -4701,6 +4743,8 @@ lp_status_t barrier_solver_t<i_t, f_t>::barrier_advanced_solve(f_t start_time, | |
| settings.log.printf("Complementarity gap (abs/rel): %8.2e/%8.2e\n", | ||
| complementarity_residual_norm, | ||
| relative_complementarity_residual); | ||
| settings.log.printf( | ||
| "Objective gap (abs/rel): %8.2e/%8.2e\n", objective_gap, relative_objective_gap); | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| settings.log.printf("\n"); | ||
| raft::copy(data.x.data(), data.d_x_.data(), data.d_x_.size(), stream_view_); | ||
| raft::copy(data.y.data(), data.d_y_.data(), data.d_y_.size(), stream_view_); | ||
|
|
@@ -4735,9 +4779,11 @@ lp_status_t barrier_solver_t<i_t, f_t>::barrier_advanced_solve(f_t start_time, | |
| primal_residual_norm, | ||
| dual_residual_norm, | ||
| complementarity_residual_norm, | ||
| objective_gap, | ||
| relative_primal_residual, | ||
| relative_dual_residual, | ||
| relative_complementarity_residual, | ||
| relative_objective_gap, | ||
| solution); | ||
| } | ||
| } | ||
|
|
||
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.
Why do we need
(!data.has_cones() && data.Q.n == 0)? I think we should have duality gap check for all kinds of problems solved by barrier.