ecboghiu / inflation

Implementations of the Inflation Technique for Causal Inference.
GNU General Public License v3.0
22 stars 3 forks source link

Wrong implementation of positive variables with semiknowns with `solveSDP_MosekFUSION` (fixed in branch) #48

Closed ecboghiu closed 2 years ago

ecboghiu commented 2 years ago

Bad and good news!

I recently found an important bug in how negative variables were being implemented with solveSDP_MosekFUSION while I was implementing the new version which will allow for bounds on variables and linear equalities and inequalities.

These are the values for the maximum minimum eigenvalue when running a feasibility problem with semiknowns with physical monomials of order 2 restricted to a maximum length of 4 for the W distribution in the quantum triangle:

I don't really see the mistake in the theory. With upper and lower bounds, the constraint in the dual formulation becomes

$\operatorname{Tr} ZF_i + L_i - U_i = 0$

In the new solveSDP_MosekFUSION, I add dual variables L and U and write that constraint by hand. In the old formulation, since there are no upper bounds, $U_i=0$, and then we are left with the variables $i$ that have lower bound $0$ have a non-zero $L_i$, therefore, we must have $\operatorname{Tr} ZF_i \leq 0$, and this is the constraint that is being implemented in this line:

https://github.com/ecboghiu/inflation/blob/b6df763e3b575f355aaf0d7b21aeff0da7f3a62e/causalinflation/quantum/sdp_utils.py#L109

The problem might be that the matrix Fi might not correspond to variable x_i, so it might have something to do with how variables_order is calculated in the presence of semiknown constraints.

I already fixed this in the new branch.

ecboghiu commented 2 years ago

Fixed in atomic-complete, latest commit is https://github.com/ecboghiu/inflation/commit/8c3f814efee735081ca54280096f19dfa99a8d06.