Skip to content

Conversation

@LucasBoTang
Copy link
Collaborator

Summary

This PR fixes an incorrect use of abs when computing the log-scaled primal-dual residual gap during adaptive restart, as noted in issue #60.

The previous implementation could invoke the integer version of abs, leading to unintended type conversion and incorrect behavior for floating-point values.

Changes

  • Replace abs(...) with fabs(...) when computing primal_dual_residual_gap in perform_restart()

@LucasBoTang
Copy link
Collaborator Author

Hi @ZedongPeng,

I ran a benchmark comparison between the current version (abs) and the test version (fabs).

download (2)

From the plot, the solve time is essentially unchanged.

The iteration counts differ slightly, and in two cases the difference exceeds 1%.

[WARNING] more than 1% mismatch in Iterations for instance ns1687037:
             Status Iterations Primal objective Dual objective
Original TIME_LIMIT  246429000      4.678306582    -250.390681
Test      TIME_LIMIT  252143200      4.689183513   -249.5551493
--------------------------------------------------------------------------------------------------

[WARNING] more than 1% mismatch in Iterations for instance neos-4535459-waipa:
             Status Iterations Primal objective    Dual objective
Original  TIME_LIMIT    7857200   3.06017374e+15  -4.356248758e+10
Test     TIME_LIMIT    7895200  3.056019324e+15  -4.372484483e+10
--------------------------------------------------------------------------------------------------

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant