inplace QR #20
inplace QR #20
Conversation
|
I like the idea very much. Can you run benchmarks that compare the old and the new version? |
|
I just encountered a bug in QRupdate.jl and filed an issue there. I have a somewhat quick fix for it but it makes it a bit less memory efficient(still better than the original dynamic qrupdate). I can open up a PR there and fix it but I just emailed Nicolás Barnafi(the person who wrote the in-place qrupdate code) and he told me he's working on an updated implementation of this and it might fix the issue + keep it memory efficient. So I'm waiting on that. |
|
@tinatorabi — I see that the unit tests are still failing with the latest commit. Please let me know if I can help. |
|
Hi! |
|
Hi! Now that the new QRupdate version is merged, everything is working just as we wanted and all the CIs just passed! I’ve updated both BPdual and omp to use in-place QR operations. :) @mpf, I think you can go ahead and merge this! |
| work = rand(n) | ||
| work2 = rand(n) | ||
| work3 = rand(n) | ||
| work4 = rand(n) | ||
| work5 = rand(m) |
There was a problem hiding this comment.
Presumably rand is being used to initialize these work arrays, and random numbers aren't really required? If so, replace with undefined allocations of the correct type.
| @@ -171,6 +178,8 @@ function bpdual( | |||
| numtrim = 0 | |||
| nprodA = 0 | |||
| nprodAt = 0 | |||
| cur_r_size = 0 | |||
There was a problem hiding this comment.
Is there a situation in which the variables cur_r_size and nact different? My understanding is that the size of R always corresponds to the number of active variables, indicated by nact. If so, would it be possible to instead use nact everywhere?
Also, it seems that on a warm-start entry, cur_r_size=0, which may be incorrect.
I have incorporated the in-place QR add/del operations in bpdual. However, I suspect the CI tests will most likely fail because the updates to QRupdate.jl have not yet been released as a new version. @mpf would it be possible for you to create a new release of QRupdate? Thanks.