AEON FVS parity option seems to have no effect

[jcrozum]

I was trying to make an FVS example for the docs, and I just can't seem to make the parity parameter do anything. Here's and example:

>>> import balm
>>> sd = balm.SuccessionDiagram.from_bnet("""
    A, B
    B, A
    C, D
    D, !C""")
>>> balm.interaction_graph_utils.feedback_vertex_set(sd.network, parity="positive")
['A', 'C']
>>> balm.interaction_graph_utils.feedback_vertex_set(sd.network, parity="negative")
['A', 'C']
>>> balm.interaction_graph_utils.feedback_vertex_set(sd.network)
['A', 'C']

Here, A <--> B is a positive feedback, and C <--| D is a negative feedback. There are no other feedback loops. I expected to see that specifying parity="positive" would return ['A'] and parity="negative" would return ['C']. Of course, I know it's not 100% guaranteed to work, so maybe I've just been unlucky with my small examples. @daemontus do you have a good example I could use here? Or is this feature not fully implemented right now?

[daemontus]

Ah, that's my fault! I added the cleanup_network routine (succession_diagram.py, line 115) to get rid of unnecessary constraints on the interaction graph... but... these are not unnecessary :D These are used as the basis for the signed interaction graph. So, long story short, in the current state, the FVS is the same because it sees all edges as neither positive nor negative. It should just be possible to fix cleanup_network by properly inferring the sign constraints there instead of erasing them.

[jcrozum]

Can I just remove this:

reg["sign"] = None

? Or will we actually need to re-infer signs somehow?

[daemontus]

We need to actually infer the signs, because they are not in .bnet files, so we need to get them from somewhere. I'll push it in a minute :)

[daemontus]

This should now be resolved on main due to #103. Hopefully it will also help a bit with attractor detection in general.

[jcrozum]

@daemontus Does resolving this issue have any impact on comments you left in SuccessionDiagram.node_nfvs about the parity option being too costly?

[daemontus]

It is still true that for very large networks (e.g. 2000+), true parity FVS computation is costly due to the cycle detection algorithm being non-linear (I did the testing for this outside of balm on models that had correct sign annotations to start with). So I think these comments are still valid :)

[jcrozum]

OK, just double checking; thanks.