-
Notifications
You must be signed in to change notification settings - Fork 15
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
WIP: use new primal-dual format (linear objectives only) #62
Conversation
working on improving the linear system solve scheme. probably will try the double QR method that CVXOPT uses. in Julia we have the advantage that if A' is sparse, qr(A') will dispatch to SuiteSparse's sparse QR, otherwise it will use LAPACK's dense QR. CVXOPT only uses dense QR. if I can get the rank of A from qr(A') then that kills another bird with the same stone, namely checking the rank condition for A in the load_data! function. Plus rank(A) currently fails if A is sparse because rank tries to do an SVD and there's no sparse SVD. |
getting close! should be able to merge in a day or two, then I can work on MOI integration #56 and have access to more tests |
I have reduced allocations down about as much as possible. there is still a big performance gap between master and this PR. some slowdown is expected because all of the current examples are formulated with variable cones, so the old format was ideal for them. I should be able to bridge that gap more by being more clever about matrix types (eg allowing uniformscaling / identity-like matrices for G, not converting sparse to dense in some places). |
src/nativeinterface.jl
Outdated
@. dir_tx = res_tx | ||
@. dir_ty = res_ty | ||
@. dir_tz = -tz | ||
@. dir_ts = res_tz |
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.
Didn't know about this macro. What is the difference between @. a = b
and a .= b
?
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.
No difference. Better when there's a minus sign though, like on 476, it seems.
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.
Better in which sense ? Less time ? Less allocation ?
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.
Allocations
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.
Then it might not allocate the intermediate vector and directly store it in the result, nice !
this has probably been the hardest part so far but finally it seems to be working. this PR will make it much easier to hook up to MOI by allowing constraint cones rather than only variable cones, and will reduce the size of the linear systems that need to be solved so will improve performance.
replaces #60
fixes #29
see discussion in #29 for evolving ideas on the new format