Skip to content

Expire CVXOPT Dependency #588

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

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Expire CVXOPT Dependency #588

wants to merge 2 commits into from

Conversation

coreyostrove
Copy link
Contributor

This finishes the expiration of the dependency on CVXOPT that we announced in the release of 0.9.13. The cvxpy dependency has had an associated minimum version requirement added.

See #437 for more.

Switches to using Clarabel as the solver for diamond distance and adds a minimum version pin for cvxpy.
@coreyostrove coreyostrove added this to the 0.9.14 milestone Jun 17, 2025
@coreyostrove coreyostrove requested review from a team as code owners June 17, 2025 04:55
@coreyostrove coreyostrove requested review from sserita and removed request for sserita June 17, 2025 04:55
Comment on lines 325 to 329
try:
prob.solve(solver='CVXOPT')
prob.solve(solver='Clarabel')
except _cvxpy.error.SolverError as e:
_warnings.warn("CVXPY failed: %s - diamonddist returning -2!" % str(e))
return (-2, _np.zeros((dim, dim))) if return_x else -2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do the following instead

    objective_val = -2
    varvals = [_np.zeros_like(J), None, None]
    sdp_solvers = ['MOSEK', 'CLARABEL', 'CVXOPT']
    for i, solver in enumerate(sdp_solvers):
        try:
            prob.solve(solver=solver)
            objective_val = prob.value
            varvals = [v.value for v in vars]
            break
        except (AssertionError, _sdps.cp.SolverError) as e:
            msg = f"Received error {e} when trying to use solver={solver}."
            if i + 1 == len(sdp_solvers):
                failure_msg = "Out of solvers. Returning -2 for diamonddist."
            else:
                failure_msg = f"Trying {sdp_solvers[i+1]} next."
            msg += f'\n{failure_msg}'
            _warnings.warn(msg)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I left almost the same comment verbatim in some other GitHub interface. I'm not sure where that comment went.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the import for _sdps?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, whoops. It's on a branch where I made an sdptools.py file. All that matters is that _sdps.cp.SolverError is cvxpy.SolverError. So it'll suffice to import cvxpy as cp and then replace with cp.SolverError.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I made this change with a small modification to skip producing warnings is MOSEK isn't installed, since we can assume this is pretty much always going to be the case for most users. (Read: I don't have MOSEK and didn't want to get constant warnings about that...)

While going through I also noticed that there was a flag disabling the diamond distance unit test on windows that was added about 3 years ago. The commit message on that indicated it was due to random CVXOPT failures on the runners. I'm going to blindly assume that the changes we're making plus the intervening three years probably fixed things on their own so reenabled those.

Add in multiple solver options that are sequentially tried before fully failing. Also turn back on diamond distance tests on windows which were previously being skipped.
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.

2 participants