vprusso/toqito

Ensure PSD operators are being optimized over proper space

Closed this issue ยท 10 comments

Refer to this comment (and the linked thread as well for context):
#420 (comment)

I came here to report the same problem, but I see that you are already working on it and it is probably solved now.

Even if this should be solved in your picos code, I suspect the current cvxpy code contains a bug:

measurements.append(cvxpy.Variable((dim_x, dim_x), PSD=True))

I think this creates PSD symmetric real-valued matrix variables, not complex hermitian matrices, so the optimization is over the wrong set. To obtain the correct solution I think one should create hermitian variables with hermitian=True and then add the measurements[i] >> 0 constraint for each.

Ah, out of curiosity, may I ask the reason to prefer picos over cvxpy? I'm old-school and I come from MATLAB's CVX plus QETLAB, but it seems they have comparable performances now: https://cgois.github.io/posts/cvx_vs_picos/

Specifically, anywhere that we use the pattern of cvxpy.Variable(... PSD=True) instead of hermitian=True and >> 0 needs to be replaced.

Thanks to falbarelli for bringing this up.

Hi,

I started making the changes, but I noticed that some cost functions need to be real for cvxpy to be able to maximize them. Specifically in the CHSH nonlocal game. Are you sure every PSD matrix should be turned into a complex valued matrix?

Hi @golanor,

Thank you for your interest in this task! Yes, I believe this should be the case (however, I would ensure that the test cases pass when making these changes).

While the matrix is complex, we don't expect the objective value to be so, so I believe this should be expected.

Let me know if you have any other questions, and thanks again!

So should I just cast the cost functions as real or should I take their absolute value or something similar?

If you look elsewhere in the toqito project, one of the strategies is to wrap the objective function within something like cvxpy.real(...). I believe this is the approach we would want to take for the offending cases (again, modulo that doing so doesn't cause any breaking test results).

Let me know if you have any additional questions, and happy hacking, @golanor ! Thank you again for your interest in this!

@vprusso Thanks, Please see my PR #600 . The "Symmetric Extension Hierarchy" tests currently do not pass, and I'm not sure why. Can you point me as to how to fix this?

@golanor The unit tests for the functions you changed are failing.

If I look at the test output, it is telling me that there is at least 1 test failing in the following files:

  • toqito/nonlocal_games/tests/test_extended_nonlocal_game.py
  • toqito/state_opt/tests/test_symmetric_extension_hierarchy.py

You can also see which test is failing because the name of the failing test is provided after ::. Hope this helps!

Thanks, that's not what I meant :)

I've looked into the tests - the test_bb84_commuting_value_upper_bound test fails due to being less accurate - the difference between the theoretical value and the result is 10^-5. Could it be that the complex number solution is less accurate than the real one to this extent?

It's the same issue with the symmetric extension hierarchy tests that fail.

What do you recommend doing?

If it's just a matter of accuracy and the threshold is 10^{-5} (which isn't bad, although 10^{-6} would be better) then I think just updating the tests with that newer threshold should be okay.

So long as the threshold changes don't require anything too drastic, I think that is an acceptable range of accuracy.

Hope that helps!

Thank you for being so communicative!

PR #600 is ready for review.

@golanor You seem to have missed one failing test.

toqito/state_opt/tests/test_symmetric_extension_hierarchy.py::test_symmetric_extension_hierarchy_extremal_werner_states