Implement bisection for gradient proj of power cone #84
Conversation
src/projections.jl
Outdated
| const DEFAULT_POWER_CONE_MAX_ITERS_NEWTON = 100 | ||
| const DEFAULT_POWER_CONE_MAX_ITERS_BISSECTION = 1_000 | ||
| const DEFAULT_POWER_CONE_TOL_CONV = 1e-7 | ||
| const DEFAULT_POWER_CONE_TOL_IN_CONE = 1e-10 |
There was a problem hiding this comment.
I did not optimize these a lot.
src/projections.jl
Outdated
| r = if isnan(r_newton) || abs(Φ_newton) > abs(Φ_bissection) | ||
| r_bissection | ||
| else | ||
| r_newton | ||
| end |
There was a problem hiding this comment.
I think we should use the best we have.
There was a problem hiding this comment.
Yes, I'm guessing it's always going to be bisection with Float64 because it can go all the way to epsilon precision but for BigFloat, Newton could win ^^
|
We should add at least one test where Newton was failing |
|
you mean explicitly? Because bisection is being called a few times. |
| Φ_bisection = _power_cone_system(r_bisection, x, y, z, α) | ||
| if abs(Φ_bisection) > tol | ||
| # This happens for instance for | ||
| # `_solve_system_power_cone([-10, 10, 1e-3], MOI.PowerCone(0.15))` |
There was a problem hiding this comment.
An example like this used to throw the warning because Newton was just returning 0 or abs(z). I'm surprised the tests were actually passing. The bisection does much better on these so we should have tests that fail for these and pass thanks to this PR
I had a lot of warnings when running the tests, the bisection method should solve them. Bisection is also the method recommended by the paper.