CASCI-lab / CANA

CANAlization: Control & Redundancy in Boolean Networks
https://casci-lab.github.io/CANA/
MIT License
22 stars 15 forks source link

Two Symbol Permutation Testing #21

Closed hsizek closed 2 years ago

hsizek commented 3 years ago

In the previous version of permutation tests, the test only made pairwise comparisons of the two symbol schema and compared that against the prime implicates. This meant that for two symbol schema with three or more elements, or two different symmetrical components, that the two symbol schema could have more Prime Implicates than reality. This change checks and makes sure that the two symbol schema is contained within the prime implicates. This configuration should not be used when calculating the input symmetry, K_s, as it will produce a severe under estimate of K_s, but it should be used when calculating the DCM, as the DCM should be a faithful representation of the logic gate. As such, I've added in an argument that gets passed through starting with the node's canalizing map function, to the two symmetry calculation. I did not add handling to check if the two symbol was produced under which algorithm (for example, someone could call check compute two symbol and then call to produce a DCM and the result would be different than just calling DCM, similarly saved data does not know which algorithm was used).

This resolves issue #20

rionbr commented 3 years ago

Hey @hsizek, I'm testing this PR, but I think we are overlooking something here.

On your canalization.boolean_canalization.py file, if I simply comment your additions to lines 277-281 I get the same result, which means whatever is inside the forDCM if statement does not make any difference. The giveaway was that you are using perm_groups instead of allowed_perm_groups. What happens is that the TS variable is not being populated and the two-symbols are included by a check that all PI are being covered ---which they aren't and therefore are all being added to the resulting TS.

Let's keep this conversation going, I think it might be easier to use all PI in your DCM computation.

hsizek commented 3 years ago

Two things:

  1. This code will only "do something" when you have a symmetry that is 4 or longer or where there are multiple symmetrical groups within the statement. The current algorithm doesn't produce ambiguous results when there are only 3 members of the symmetrical group and only one symmetrical group. You should see differences with the example from issue #20.
  2. I'm using allowed_permutation_groups differently than in check_schemata_permutations_v2. I just use it as a boolean, where you use it to store more information. I would have to think about it more to determine if there are cases in which the permutation groups would fail (where it would be better to use all PIs as you suggest).

The issue that I'm trying to address: permutation test (v2) only tests pairwise exchanges, when there could be multiple changes happening at one time, this means that a two symbol schemata can be accepted when it has implicates that are invalid (belong to the other state of the node), as the pairwise test doesn't actually check all possible combinations. This was the most logical way that I saw to implement it to mirror the current code flow.

rionbr commented 3 years ago

I understand the issue you are trying to address, I just don't think the current implementation is the best way to solve it. :-)

I've been talking to Luis about this and we are going to try to redo the "manual algorithm" (with pen and paper) to check where these special cases are slipping through – whether the original algorithm doesn't account for them or there is something wrong with the code. While I'm going through the code I'm also trying to optimize and simplify it a bit more.

Meanwhile, I will leave this PR open. Cheers,