-
Notifications
You must be signed in to change notification settings - Fork 18
Warm start in Eigen interface #95
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
8297098
2502607
6c562a7
95b7eb8
6f84dc5
6eccbc5
05f5f15
213c4c5
3003e3b
0954931
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,12 +15,12 @@ EigenDAQPResult::EigenDAQPResult(int n, int m) | |
| } | ||
|
|
||
| void EigenDAQPResult::resize_primal(int n) { | ||
| x_.resize(n); | ||
| x_.conservativeResizeLike(Eigen::VectorXd::Zero(n)); | ||
| x = x_.data(); | ||
| } | ||
|
|
||
| void EigenDAQPResult::resize_dual(int m) { | ||
| lam_.resize(m); | ||
| lam_.conservativeResizeLike(Eigen::VectorXd::Zero(m)); | ||
| lam = lam_.data(); | ||
| slack_.resize(m); | ||
| } | ||
|
|
@@ -136,6 +136,7 @@ EigenDAQPResult daqp_solve(Eigen::Matrix<double, Eigen::Dynamic, Eigen::Dynamic, | |
| DAQP::DAQP(int max_variables, int max_constraints, int max_constraints_in_level) | ||
| : is_solved_{false} | ||
| , is_slack_computed_{false} | ||
| , warm_start_{false} | ||
| , max_variables_{max_variables} | ||
| , max_constraints_{max_constraints} | ||
| , max_constraints_in_level_{max_constraints_in_level} { | ||
|
|
@@ -215,13 +216,39 @@ int DAQP::update(Eigen::MatrixXd const& H, | |
| int resize_status = resize_result(n, m, break_points); | ||
| assert(resize_status == 0); | ||
|
|
||
| qp_ = {n, m, ms, H_ptr, f_ptr, A_ptr, bu_ptr, bl_ptr, sense_ptr, bp_ptr, n_tasks}; | ||
|
|
||
| if (update_mask < 0) { | ||
| // Assume that everythig should be updated | ||
| update_mask = UPDATE_Rinv + UPDATE_M + UPDATE_v + UPDATE_d + UPDATE_sense + UPDATE_hierarchy; | ||
| } | ||
|
|
||
| if (warm_start_){ | ||
| if(sense_ptr == nullptr) | ||
| sense_ptr = work_.sense; // Directly work with sense of workspace | ||
| for(int i = 0; i < m; i++){ | ||
| if(result_.lam[i] > 0) | ||
| sense_ptr[i] |= 1; // Active + Upper | ||
| else if(result_.lam[i] < 0) | ||
| sense_ptr[i] |= 3; // Active + Lower | ||
| else | ||
| sense_ptr[i] = 0; | ||
| } | ||
| update_mask |= UPDATE_sense; // Ensure sense is update | ||
|
|
||
| // Adjust break points based on feasibility of previous solution | ||
| if(A_ptr != nullptr && n_tasks > 1){ | ||
| int first_violating = daqp_first_violating(result_.x,A_ptr,bu_ptr,bl_ptr,n,m,ms,settings_.primal_tol); | ||
| for(int i = 0; i < n_tasks; i++){ | ||
| if(bp_ptr[i] >= first_violating){ | ||
| n_tasks = n_tasks - i; | ||
| bp_ptr += i; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| qp_ = {n, m, ms, H_ptr, f_ptr, A_ptr, bu_ptr, bl_ptr, sense_ptr, bp_ptr, n_tasks}; | ||
|
|
||
| int status = update_ldp(update_mask, &work_, &qp_); | ||
| is_solved_ = is_slack_computed_ = false; | ||
| return status; | ||
|
|
@@ -250,6 +277,14 @@ EigenDAQPResult const& DAQP::solve(Eigen::Matrix<double, Eigen::Dynamic, Eigen:: | |
| } | ||
|
|
||
|
|
||
| void DAQP::set_warm_start() { | ||
| warm_start_ = true; | ||
| } | ||
|
|
||
| void DAQP::set_cold_start() { | ||
| warm_start_ = false; | ||
| } | ||
|
|
||
|
Comment on lines
+280
to
+287
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. If the behavior depends on To me, the user is probably interested in using the solver class because there are more related problems to solve. Therefore, warm starting should be the preferred option. What do you think?
Owner
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. With the most recent change it should make sense to warm start by default (although, as is always the case with warm starting, it might exacerbate the worst-case solution time if the new problem is not so similar to the previously solved)
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. Basically, if we remove
Owner
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. Yes, that could work I think! I am still not fully convinced that warm starting should be the default though. It depends on if we want to improve the average or the worst-case behavior. |
||
| void DAQP::set_primal_tol(double val) { | ||
| settings_.primal_tol = val; | ||
| is_solved_ = false; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| #include <iostream> | ||
| #include <Eigen/Dense> | ||
| #include <daqp.hpp> | ||
|
|
||
| int main() { | ||
| // | ||
| double precision = 1e-5; | ||
| Eigen::MatrixXd A; | ||
| Eigen::VectorXd bu, bl; | ||
| Eigen::VectorXi break_points; | ||
|
|
||
| // Setup solver | ||
| DAQP solver(3, 50, 5); | ||
| solver.set_warm_start(); | ||
|
|
||
| // Task 1: -1 <= x <= 1 | ||
| Eigen::MatrixXd A0 = Eigen::MatrixXd::Identity(3, 3); | ||
| Eigen::VectorXd bu0 = Eigen::VectorXd::Ones(3); | ||
| Eigen::VectorXd bl0 = -Eigen::VectorXd::Ones(3); | ||
|
|
||
| // Task 2: x1+x2+x3 <= 1 | ||
| Eigen::MatrixXd A1 = (Eigen::MatrixXd(1, 3) << 1, 1, 1).finished(); | ||
| Eigen::VectorXd bu1 = Eigen::VectorXd::Ones(1); | ||
| Eigen::VectorXd bl1 = Eigen::VectorXd::Constant(1, -DAQP_INF); | ||
|
|
||
| // Task 3: x1 - x2 == 0.5 | ||
| Eigen::MatrixXd A2 = (Eigen::MatrixXd(1, 3) << 1, -1, 0).finished(); | ||
| Eigen::VectorXd bu2 = 0.5 * Eigen::VectorXd::Ones(1); | ||
| Eigen::VectorXd bl2 = 0.5 * Eigen::VectorXd::Ones(1); | ||
|
|
||
| // Task 4: 10 <= 3*x1+x2-x3 <= 20 | ||
| Eigen::MatrixXd A3 = (Eigen::MatrixXd(1, 3) << 3, 1, -1).finished(); | ||
| Eigen::VectorXd bu3 = 20 * Eigen::VectorXd::Ones(1); | ||
| Eigen::VectorXd bl3 = 10 * Eigen::VectorXd::Ones(1); | ||
|
|
||
| // ==== First setup (stack all tasks) ==== | ||
| A = (Eigen::MatrixXd(6, 3) << A0, A1, A2, A3).finished(); | ||
| bu = (Eigen::VectorXd(6) << bu0, bu1, bu2, bu3).finished(); | ||
| bl = (Eigen::VectorXd(6) << bl0, bl1, bl2, bl3).finished(); | ||
| break_points = (Eigen::VectorXi(4) << 3, 4, 5, 6).finished(); | ||
| solver.solve(A, bu, bl, break_points); | ||
| int cold_iter = solver.get_iterations(); | ||
|
|
||
| std::cout << "Solution cold start: \n"; | ||
| std::cout << solver.get_primal().transpose() << std::endl; | ||
| std::cout << "Solve time: " << solver.get_solve_time() << " seconds"; | ||
| std::cout << " | Iterations: " << solver.get_iterations() << std::endl; | ||
| if (!solver.get_primal().isApprox((Eigen::VectorXd(3) << 1, 0.5, -1).finished(),precision)) { | ||
| return 1; | ||
| } | ||
| solver.set_warm_start(); | ||
| solver.solve(A, bu, bl, break_points); | ||
| std::cout << "Solution warm start: \n"; | ||
| std::cout << solver.get_primal().transpose() << std::endl; | ||
| std::cout << "Solve time: " << solver.get_solve_time() << " seconds"; | ||
| std::cout << " | Iterations: " << solver.get_iterations() << std::endl; | ||
| if (!solver.get_primal().isApprox((Eigen::VectorXd(3) << 1, 0.5, -1).finished(),precision)) { | ||
| return 1; | ||
| } | ||
|
|
||
| // Ensure cold start reduced number of iterations | ||
| if(cold_iter <= solver.get_iterations()) return 1; | ||
|
|
||
|
|
||
| return 0; | ||
| } |
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.
Currently this should always be the case because
senseis set to empty, when we call solve with inputs.Does it mean that it should be a member of the class instead?
Or can we just always use
work_.senseand deletesense?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 should always be able to use work_.sense. Moreover, sense might be non empty if the update(...) + solve() method is used rather than just solve(...).
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.
What I mean is that in this case we can just assign
sensetowork_.senseand removesense_ptr. The latter seems redundant.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.
I think that this would not work in the case when the user calls DAQP::update with their own sense! I agree, however, that it would work if one would only call the solve with A,bu and bl, since sense is always set to null in that call.
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.
I thought we could do
work_.sense = sense.data()similarly to what we do right now in the update.Couldn't that work?
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.
work_.sense points to memory allocated internally by DAQP, so to avoid memory leaks it should not be altered.
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.
I see and it would then try to free stuff owned by the eigen variable.
Thanks for clarifying.