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

Fix complex interface #74

Merged
merged 1 commit into from
Mar 1, 2023
Merged

Fix complex interface #74

merged 1 commit into from
Mar 1, 2023

Conversation

araujoms
Copy link
Contributor

As it is the complex interface is not working, it gives the wrong answer when we use the field K.scomplex.

The problem is an error in the construction of the QR matrix: to compute 2*real(m(i,j)) it does m(i,j) - m(j,i) instead of m(i,j) + m(j,i). As such this only affects problems where the real part of the SDP coefficients are nonzero (which might be useful for testing). This PR fixes it. I'm happy to send code to demonstrate the issue, but I don't see how to attach a file to a PR.

I also fixed a typo in an error message that made me waste a lot of time with debugging.

As it is the complex interface is not working, it gives the wrong answer when we use the field K.scomplex. 

The problem is an error in the construction of the QR matrix: to compute 2*real(m(i,j)) it does m(i,j) - m(j,i) instead of m(i,j) + m(j,i). As such this only affects problems where the real part of the SDP coefficients are nonzero (which might be useful for testing). This PR fixes it. I'm happy to send code to demonstrate the issue, but I don't see how to attach a file to a PR.

I also fixed a typo in an error message that made me waste a lot of time with debugging.
@araujoms
Copy link
Contributor Author

The following code demonstrates the issue. We are minimizing trace((X+Y)*x) under the constraints that x is PSD and has trace one. The actual solution is -sqrt(2), but SeDuMi gives -1. Note that if we minimize instead trace(Y*x) the real part of the offdiagonals of the objective is zero and the bug doesn't trigger. The solution is -1 and SeDuMi gets -1.

X = [0,1;1,0];
Y = [0,-1i;1i,0];

c = vec(X) + vec(Y);
A = [1;0;0;1];
b = 1;

K.s = 2;
K.scomplex = 1;

[x,y,info] = sedumi(A,b,c,K);

real(c'*x)

@siko1056
Copy link
Member

siko1056 commented Mar 1, 2023

Thank you for the change with description @araujoms 👍

@araujoms araujoms deleted the patch-1 branch March 1, 2023 07:23
@araujoms
Copy link
Contributor Author

Would it be possible to release a version with this bugfix? I've added support in YALMIP for accessing the complex interface, but it will break people's code if we turn it on for all versions of SeDuMi. If you release a version 1.3.6 we could detect that from YALMIP, and only use the complex interface for that version onwards.

@siko1056
Copy link
Member

Yep I think to merge #75 as well and then there is enough for a patch level release. Would you in general like to work on this SeDuMi repository as maintainer?

@araujoms
Copy link
Contributor Author

I'd be glad to help, but I'm afraid I don't know enough about SeDuMi to be of much use.

It turns out YALMIP can't actually detect the solver version, it deduces the version from the files that are present. We'd need then to add a dummy file for YALMIP to detect.

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