ecboghiu / inflation

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

Creating an example of a supports test (if and when supports are working...) #65

Closed eliewolfe closed 1 year ago

eliewolfe commented 2 years ago

Right now support_problem=True is still (mysteriously!) sensitive the the actual numerical values set in set_distribution or set_values, and not just whether they are zero or strictly positive. See discussion starting at https://github.com/ecboghiu/inflation/issues/65#issuecomment-1255628172.

So: FIRST AND FOREMOST

One of the new features in the code is the ability to explore possibilistic causal inference in addition to probabilistic causal inference. For a set_distribution example it appears to be ok.

We also need to be careful regarding set_objective! The numerical values of the coefficients do not matter. An objective is only meaningful for a supports problem if all the coefficients in the objective have the SAME SIGN. Thus, if the user specifies an objective with mixed-sign coefficients, and we've specified supports_problem = True, then we need to either abort or attempt to use the equalities to get a homogenous-sign objective.

eliewolfe commented 2 years ago

A key insight in support testing is that the '1' variable should be treated as free (albeit nonnegative) instead of constant. The codebase in the devel branch was not accomodating this. I hacked a bunch of patches together to make it work, which I used to initiate a new for_supports branch. This is all collecting in https://github.com/ecboghiu/inflation/commit/f04ab7f9f7b5f09c5f7609a69c0547628912d220. However, it isn't working as expected! If I change rescale numerical values in the distribution for set distribution (i.e. I use a nonnormalized distribution), the solver starts returning infeasible! I can't understand why, however... surely all the constraints passed forward to the actual solver are IDENTICAL regardless of the normalization of the distribution, right? @ecboghiu, can you find any explanation for the inconsistency in the example I pushed to the for_supports branch in commit https://github.com/ecboghiu/inflation/commit/e4d3f6fa40622a53a365486cc44253b4a3fd222c ??

ecboghiu commented 2 years ago

I think there's a simpler solution to this. Simply changing 'CONSTANT_IDX' from '1' to something else in solve_SDP should solve all the problems. I think actually the. Eat solution is to remove all together this notion of a CONSTANT _IDX. If something is constant or not should be simply specified in known_moments and the One monomial shouldn't be treated in a special way. I'll do this changes later this afternoon.

apozas commented 2 years ago

Just wanted to say that thanks for moving the development of this issue to a new branch. This is precisely how it should be handled :smile:

eliewolfe commented 2 years ago

@ecboghiu I wish it were that simple! In the for_supports branch I do exactly what you propose, allowing CONSTANT_KEY to be a user-specifiable argument in solveSDP_MosekFUSION, which is set to Fake_1 instead of 1 when InflationSDP.supports_problem=True. It doesn't actually change the behavior of the solver enough, however, as illustrated in my BUG_MWE file in that branch.

eliewolfe commented 2 years ago

So, this MAY be resolved by the cleanup in https://github.com/ecboghiu/inflation/commit/b62168ebbee3574ab6656515a820de881f95a3ba. I can't confirm yet, because I'm getting the error line 292, in solveSDP_MosekFUSION C[i, var2index[x]] = c KeyError: '0' which @ecboghiu is working on now.

ecboghiu commented 1 year ago

Can you check if this works now?

Also, I find that in the normalisation equalities several zero monomials sneak in:

185: {<A_1_2_0 B_1_0_0 B_1_1_0>: -1, 0: -1}
186: {<A_1_2_0 B_1_1_0>: -1, 0: -1}
187: {<A_1_2_1 B_1_0_0 B_1_1_0>: -1, 0: -1}
188: {pAB(10|20): -1, 0: -1}

I believe this doesn't make sense, as it is effectively setting several moments to zero. Can you look into this @eliewolfe ?

eliewolfe commented 1 year ago

Status update: Support testing appears to be working now, following commits https://github.com/ecboghiu/inflation/commit/be46d90ea0b1c152130b9a39dd15cc97c79ff890 and https://github.com/ecboghiu/inflation/commit/9316fcc98393ea1e1b45c67ab58a2056bb2d6ec2. Certificates for support problems are not yet working, however, see https://github.com/ecboghiu/inflation/issues/72#issuecomment-1261629425 .

ecboghiu commented 1 year ago

@eliewolfe can we close this?

apozas commented 1 year ago

It all seems to work, so I am closing this.