Closed jcrozum closed 4 years ago
This is implemented in a separate branch (succesion-diagram-improvements).
Before adding a new MotifReduction ot the SuccessionDiagram object, we check to see if the new motif history is a permutation of the motif history for a motif reduction already in the diagram. If so, we keep track of the permutation in the diagram object and do not create a new MotifReduction object.
I have attached a small visual example that illustrates the old way vs the pad way. Each yellow node represents a MotifReduction object.
Feedback on whether or not this approach makes sense would be appreciated! Are there any potential pitfalls to doing it this way?
Small mistake in the figure. It's corrected below. The bracketed sequences in the diagram are permutations stored within the SuccessionDiagram object. So [[1,0]] means we can interchange the two motifs listed in the history, while, for example, [[0,1,3,2],[2,0,1,3]] would mean we could interchange the two more recent or cycle the first three.
I think the approach makes perfect sense and don't see any pitfalls. Running test5.txt (a maximally reduced version of the ABA induced closure network) illustrates the necessity of this this consolidation; there are a lot of possible successions. I did not see where the succession-diagram-improvement is, perhaps it is not part of the master branch. Running test5.txt with the two versions of the code can test the method.
succession-diagram-improvement is the name of the branch that has the new implementation (which is part of SuccessionDiagram.py). I haven't merged it yet because there were a few more things I wanted to implement and test there.
Overall, I think this would be extremely helpful, given that we know motif permutation is a problem when trying to understand the succession diagram. A possible concern I am thinking on is whether the methodologies to get the control sets from the succession diagram would need to be adapted (and how) to account for this merging of branches in the succession diagram. Because of this, it might be worth not eliminating the option for the old version of the succession diagram.
I'm pretty sure that if we store the succession diagram as I described, we can reconstruct the full diagram if we really need to (abstractly, if possible). I'd really like to avoid storing copies of MotifReduction objects for every possible permutation of compatible stable motifs because this scales factorially with the number of stable motifs.
I'd rather adapt the control set algorithm if it isn't too difficult. I think we will have to adapt the algorithm somewhat either way. Do you foresee any specific difficulties with running the algorithm on this data type?
I think paths up the tree from attractors back to the roots are all preserved. Is there a situation in which this might not be the case?
I was trying to think if there was situation where a problem arises, but I couldn't think of it on top of my head.
Given that, like you say, all paths up the tree and down the tree are preserved, then that should be enough to avoid problems. Given this, I think you're right it's better to just keep this newer version of the succession diagram and come back to the old version only if needed (the old commits are also still saved, so that shouldn't be a problem).
Okay, I will close this issue and we can revisit this if we run into any problems with the attractor reprogramming.
For example, motif1 -> motif2 -> motif3 and motif2 -> motif1 -> motif3 should not result in two copies of motif3, but I think it does right now.