Skip to content
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

Regression from ECOS causing CVXPY test failures #120

Open
PTNobel opened this issue Jun 23, 2024 · 2 comments
Open

Regression from ECOS causing CVXPY test failures #120

PTNobel opened this issue Jun 23, 2024 · 2 comments

Comments

@PTNobel
Copy link

PTNobel commented Jun 23, 2024

We were investigating some of the test case failures as we migrate the CVXPY test suite from ECOS to Clarabel in cvxpy/cvxpy#2479

It seems at least some of the cases are related to when ECOS correctly reports PRIMAL INFEASIBLE, but Clarabel reports InsufficientProgress instead.

This breaks our disciplined quasiconvex programing work and is consistent with issues that the EE 364a course staff had with Clarabel when assigning bisection methods in the homework.

Frankly, I'm a little worried about what to do here; the 1.5.x CVXPY release apparently regresses the DQCP support compared to prior releases. I'm not sure if this is just an issue of different default feastols or if this is from a new infeasibility detection approach.

Here's a minimized example:

import scipy.sparse as sp
import numpy as np

def make_array(d):
    out = sp.dok_matrix((8, 3))
    for (i, j), v in d.items():
        out[i, j] = v
    return out.tocsc()

G = make_array({
    (0, 0): -1.0,
    (2, 0): 1.0,
    (4, 0): -1.0,
    (1, 1): -1.0,
    (3, 1): 1.0,
    (5, 1): -1.0,
    (6, 1): -1.0,
    (4, 2): 72.001953125,
    (5, 2): -1.0,
    (6, 2): 1.0})
h = np.array([ 0.,  0., 12.,  6.,  0.,  0.,  0.,  2.])
c = np.array([0., 0., 0.])
nonneg = 5
soc = [3]

import ecos

solution = ecos.solve(c, G, h, {'l': 5, 'q': [3]})


import clarabel

P = sp.dok_matrix((3, 3)).tocsc()

cones = [clarabel.NonnegativeConeT(5), clarabel.SecondOrderConeT(3)]
settings = clarabel.DefaultSettings()
solver = clarabel.DefaultSolver(P,c,G,h,cones,settings)
solution = solver.solve()
@mipals
Copy link

mipals commented Jun 26, 2024

Having looked the code I do not think there is anything to worry about. It seems as the old infeasibility checks are still present. Adding the ones from the preprint seem to fix the issue (see here).

The only thing that is not really described in the preprint is a multiplication with 1000, which Paul also mentions in a comment in the code should be fixed

//PJG hardcoded factor 1000 here should be fixed

I am not sure what fix Paul was thinking of, but I simply removed the multiplication.

@mipals
Copy link

mipals commented Jun 28, 2024

Having looked at this again, I can see that I missed the scalings that happens in the updating of info. As such the above is not really a fix. However, I did notice that a normx is missing here

self.res_primal_inf = residuals.rx_inf.norm_scaled(dinv) / T::max(T::one(), normz);

For the simple problem here, it does seem like the big reason that it fail is due to the multiplication of 1000, as the ktratio never gets above 1e9 before InsufficientProgress is reported.

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

No branches or pull requests

2 participants