-
Notifications
You must be signed in to change notification settings - Fork 12
[WIP] Integrate PSLP Presolver #41
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
base: main
Are you sure you want to change the base?
Conversation
src/presolve.c
Outdated
| if (!reduced_result || !info->presolver) return NULL; | ||
|
|
||
| int n_red = info->presolver->reduced_prob->n; | ||
| double *z_dummy = (double*)calloc(n_red, sizeof(double)); |
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.
The function "postsolve" recovers optimal primal and dual variables of the original problem. For the dual variables to be recovered correctly, the optimal dual variable z for the reduced problem must be correct. So this will likely result in the wrong dual variables for the original problem. However, the recovered primal variables will be correct.
|
I think this looks correct, with one small exception (see my comment above). At the moment, I don’t have a clear hypothesis about whether the presolved problem should require fewer or more iterations. Whenever you have the opportunity to run the benchmarks, it would be very interesting to hear your insights on the following:
|
|
I thought of something else. When PSLP fixes variables, it updates an offset in the objective. The offset is stored in presolver->prob->obj->offset. I think it makes sense to include this offset when evaluating the termination criteria inside cuPDLPx for the reduced problem. |
@dance858 It might be better to add the definition of |
|
Good point and I totally agree. I'm pushing up the fix now. The offset is stored in PresolvedProblem->obj_offset. |
src/presolve.c
Outdated
| void sanitize_infinity_for_pslp(double* arr, int n, double pslp_inf_val) { | ||
| for (int i = 0; i < n; ++i) { | ||
| if (isinf(arr[i]) || fabs(arr[i]) >= pslp_inf_val) { | ||
| arr[i] = (arr[i] > 0) ? pslp_inf_val : -pslp_inf_val; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #define CUPDLP_INF std::numeric_limits<double>::infinity() | ||
|
|
||
| void restore_infinity_for_cupdlpx(double* arr, int n, double pslp_inf_val) { | ||
| for (int i = 0; i < n; ++i) { | ||
| if (arr[i] >= pslp_inf_val * 0.99) { | ||
| arr[i] = INFINITY; | ||
| } | ||
| else if (arr[i] <= -pslp_inf_val * 0.99) { | ||
| arr[i] = -INFINITY; | ||
| } | ||
| } | ||
| } | ||
|
|
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 this the correct and best way to handle PSLP_INF? @dance858
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.
I think it makes sense. PSLP_INF is defined internally as 1e20. cuPDLPx seems to use a built-in definition of infinity? I might be able to update PSLP to use INFINITY defined in math.h. Do you think that makes sense?
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.
Yeah, I think that makes more sense. It’ll make this integration and future ones much easier.
|
Hi @dance858 . I have updated the |
Description
This PR adds optional support for the PSLP presolver in
cuPDLPx. When enabled with-p, the problem is presolved withPSLPbefore being sent to the GPU.Summary
The integration works correctly, and
PSLPsignificantly reduces the problem size (rows/cols/nnz). However, the presolved problem currently takes more PDHG iterations to converge.Any feedback on whether this behavior is expected with
PSLPwould be appreciated. @dance858Output
Without PSLP presolve.
With PSLP presolve