osqp / osqp-matlab

Matlab interface for OSQP
https://osqp.org/
Apache License 2.0
42 stars 25 forks source link

Different P and A sizes cause Matlab to crash #24

Closed markosvec closed 4 years ago

markosvec commented 4 years ago

When passing matrices P ([n x n]) and A ([m x n_1]) to problem setup function, if one accidentally defines matrix A in a way that n_1 != n (false optimizer size), Matlab crashes instead of reporting an error. In case no one reported it earlier, I think this is something which could easily be fixed and would improve the usability of the software.

imciner2 commented 4 years ago

Are you using the current master branch version of this repository, or the released 0.6.0 zip file? I believe there was a change after 0.6.0 was released that might fix this.

goulart-paul commented 4 years ago

I have just tried this on v0.6 and can't reproduce the error. If you still see the error on v0.6, please post a MWE and we will make a fix.

markosvec commented 4 years ago

I also have v0.6 and in cases when P and q are of the same size, but different from A, Matlab simply crashes. I have given this to two of my colleagues and they replicated my error. Here is the code example:

P = ones(6);
q = ones(6,1);
A = rand(10,5);
low = rand(10,1);
upp = low + rand(10,1);

prob = osqp;
prob.setup(P, q, A, low, upp);
goulart-paul commented 4 years ago

Strange. I have just tried this myself and the matlab interface catches the error, i.e. I get

Error using osqp/setup (line 208) Incorrect dimension of A

Line 208 of osqp.m is this:

assert(size(A, 2) == n, 'Incorrect dimension of A');

which seems like it should catch your error. Can you put a breakpoint there and see if it gets that far?

Have you perhaps pulled the v0.6 version but not rebuilt the solver using the make_osqp.m script?

If none of the above helps, can you post the OS and matlab version you are using?

imciner2 commented 4 years ago

That error handling was added after 0.6.0 was released, so the source zip file of 0.6.0 doesn't include them. To get the error handling you need to use the current master branch - hence my question about if they were using master or the source zip.

goulart-paul commented 4 years ago

@imciner2 OK, I see the problem. In that case just grabbing the file osqp.m direct from the git master should solve the problem, since it doesn't look like any of the C-side mex interface changed when that update was made.

markosvec commented 4 years ago

You were both right. It solves the problem. Thank you for your answers :)