ecboghiu / inflation

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

modified noncommutation matrix is completely ignored by InflationLP #150

Open eliewolfe opened 6 days ago

eliewolfe commented 6 days ago

Describe the bug The main distinction between fanout inflation and nonfanout inflation is in terms of the events which are enforced to have nonnegative probability. A multipartite event (tuple of single-partite events) is valid if all the single-partite events within it can occur simultaneously. This is exactly equivalent to "all the single partite events pairwise commute and no pair is orthogonal". In summary: Classically, everything commutes, and so all events aside from orthogonal pairings of single-partite events are valid multipartite events. With nonclassical sources, we additionally need to exclude multipartite events which would involve a noncommuting pair of single-partite events. As such, we would expect InflationLP to make extensive use of InflationProblem._default_noncomm. But this is not the case! Instead of using the noncommutation matrix to generate the valid events, InflationLP calls InflationProblem._generate_compatible_monomials_given_party. (See InflationLP and https://github.com/ecboghiu/inflation/blob/5eba90f00dd349371669c4b7b719610237313282/inflation/lp/InflationLP.py#L1373 works fine so long as the commutation relations are just the default (everything commutes except for two operators both associated with the same party, where the two operators have partially-overlapping copy indices or differing settings.) I'm quite interested in studying scenarios where we modify the commutation relations, however. In these cases, the LP completely fails. Additionally, even inf _default_noncomm is modified manually, certain properties of InflationProblem fail to get updated, as they are fixed during __init__, namely _compatible_measurements.

Proposed Fix

  1. Do not compute _compatible_measurements during __init__. Instead, make it an on-the-fly computed @property of InflationProblem, such that it will depend on the modified _default_noncomm.
  2. Completely scrap _generate_compatible_monomials_given_party (and its two helper functions _subsets_of_compatible_mmnts_per_party and _party_positions_within_lexorder).
  3. Have InflationLP use _compatible_measurements to construct the set of all valid multipartite events, instead of using _generate_compatible_monomials_given_party. See, maximal cliques on this compatibility graph would the maximal-length valid events! These generate the nonnegativity inequalities for the LP. The actual monomials would be constructed by first excluding any events which involve a final-outcome (for childless variables) and then closing the set under taking subsets.
eliewolfe commented 5 days ago

This bug was much worse than I realized! Aside from the edge-case of a user specifying a custom noncommutation matrix, InflationLP would also give incorrect results (false reports of incompatability) when certain operators associated with different parties would not commute, such as whenever we have nonclassical source(s) and intermediate latent(s). I fixed this in commit 86c98eb . (Potentially significant future performance improvement is possible by custom-writing a clique-finding algorithm that simultaneously reports all cliques in additional to maximal cliques. At the moment we construct the set of all cliques from the set of maximal cliques by closing under powerset, which is inefficient.)

eliewolfe commented 10 hours ago

Performance enhancement to close-out this issue has now been implemented in b86759. Perhaps some testing should be done to check the relative performance of the two all_and_maximal_cliques algorithm in utils.py? @apozas