Skip to content
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

undocumented behavior of osqp_warm_start_x and osqp_warm_start_y #62

Closed
mlubin opened this issue May 3, 2018 · 3 comments
Closed

undocumented behavior of osqp_warm_start_x and osqp_warm_start_y #62

mlubin opened this issue May 3, 2018 · 3 comments

Comments

@mlubin
Copy link
Contributor

mlubin commented May 3, 2018

I was a bit surprised to discover that osqp_warm_start_x clears the dual warm start and osqp_warm_start_y clears the primal warm start:
https://github.com/oxfordcontrol/osqp/blob/370dd33622aa1fec1baae3881d7a5a56bf0ebb19/src/osqp.c#L899-L900

https://github.com/oxfordcontrol/osqp/blob/370dd33622aa1fec1baae3881d7a5a56bf0ebb19/src/osqp.c#L918-L920

This isn't documented, and it's natural to assume that these functions would just act like setters for the primal and dual warm starts.

@bstellato
Copy link
Collaborator

You are right, it is not clear from the docs. We chose to do so because if the warm_start setting is on, we do not cold start any variable. See https://github.com/oxfordcontrol/osqp/blob/bb5bb360b6dee9725256cb6953c774a82520edda/src/osqp.c#L330-L331

If we remove the lines you mentioned, we could have some issues. For example, if someone warm starts x and not y, osqp would use an uninitialized variable for the first y (unless the osqp_solve function was called before. In that case it would use y from the previous solution). This might also be the desired behavior if we want the user to set only x or only y.

One alternative would be to cold start the variables x, y and z in the osqp_setup function. However, this is usually an unnecessary operation since it happens already in osqp_solve.

@mlubin
Copy link
Contributor Author

mlubin commented May 4, 2018

My main point is that from reading the documentation (and inferring from the function names), it's reasonable to expect that

osqp_warm_start_x(work, x);
osqp_warm_start_y(work, y);

is equivalent to

osqp_warm_start(work, x, y);

which is not the case. This easily introduces subtle bugs because you have to pay close attention to check if a warm start is properly used.

@bstellato
Copy link
Collaborator

I have fixed the problem in this commit 47043fe. The behavior should be more clear now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants