-
Notifications
You must be signed in to change notification settings - Fork 25
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
included update_warm_start functionality #37
base: master
Are you sure you want to change the base?
Conversation
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.
Sorry for the delay in reviewing this. I just had two comments on the contents of this.
@@ -299,6 +299,34 @@ void mexFunction(int nlhs, mxArray *plhs[], int nrhs, const mxArray *prhs[]) | |||
return; | |||
} | |||
|
|||
// update warm_start | |||
if (!strcmp("update_warm_start", cmd)) { |
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.
if (!strcmp("update_warm_start", cmd)) { | |
if (!strcmp("warm_start", cmd)) { |
The class wrapper doesn't call update_warm_start
actually, it will call warm_start
when it wants to update both vectors.
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.
Actually, I may have gotten confused here because I didn't see this was the emosqp
file, so this change probably isn't actually needed (although we need to document this now in https://github.com/oxfordcontrol/osqp/blob/master/docs/codegen/matlab.rst as well).
y_vec = copyToCfloatVector(mxGetPr(y), (&workspace)->data->m); | ||
} | ||
|
||
if(!mxIsEmpty(y)){ |
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.
Is there a reason the call to warm starting is guarded against y being non-empty? As another note, with the update above to make this function callable from the wrapper, it should be guaranteed to get both an x and y to warm start.
dear Ian,
the emosqp interface does indeed not offer the same functionality compared to the "class based" Matlab interface of OSQP.
best regards,
Raoul
De : Ian McInerney ***@***.***>
Envoyé : vendredi 28 mai 2021 21:27
À : oxfordcontrol/osqp-matlab ***@***.***>
Cc : Herzog Raoul ***@***.***>; Author ***@***.***>
Objet : Re: [oxfordcontrol/osqp-matlab] included update_warm_start functionality (#37)
@imciner2 commented on this pull request.
________________________________
In codegen/files_to_generate/emosqp_mex.c<#37 (comment)>:
@@ -299,6 +299,34 @@ void mexFunction(int nlhs, mxArray *plhs[], int nrhs, const mxArray *prhs[])
return;
}
+ // update warm_start
+ if (!strcmp("update_warm_start", cmd)) {
Actually, I may have gotten confused here because I didn't see this was the emosqp file, so this change probably isn't actually needed (although we need to document this now in https://github.com/oxfordcontrol/osqp/blob/master/docs/codegen/matlab.rst as well).
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<#37 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ATZN2G7NBHDTNKWBN53YWG3TP7U7JANCNFSM43MXTL3Q>.
|
Sorry about that confusion. My second comment on the if statement guarding the call to the actual OSQP update function still needs to be addressed though, since only checking for y non-empty seems to be incorrect. That function requires both x and y, so we should be making sure they both are non-empty for the call and throwing an error if either are empty I believe. |
Hi Ian,
in principle a warm start with only x and without y is possible, but indeed the question is whether it makes sense ...
By the way, the same question applies to the
if (!strcmp("update_bounds", cmd))
a couple of lines above. Does it make sense to provide only l without u ?
Do you plan a new release of osqp in the next future ?
best regards from Western Switzerland,
Raoul
De : Ian McInerney ***@***.***>
Envoyé : dimanche 30 mai 2021 15:30
À : oxfordcontrol/osqp-matlab ***@***.***>
Cc : Herzog Raoul ***@***.***>; Author ***@***.***>
Objet : Re: [oxfordcontrol/osqp-matlab] included update_warm_start functionality (#37)
Sorry about that confusion. My second comment on the if statement guarding the call to the actual OSQP update function still needs to be addressed though, since only checking for y non-empty seems to be incorrect. That function requires both x and y, so we should be making sure they both are non-empty for the call and throwing an error if either are non-empty I believe.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<#37 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ATZN2G74II4Z7HTXT7HH7XLTQI4VZANCNFSM43MXTL3Q>.
|
The function to call if you only want to warm start one of them is |
We plan to remove |
Basically what I am saying we should do is have code before the call to
because otherwise we can get a segfault/random data if one isn't specified. |
No description provided.