ELIFE-ASU / Neet

Simulating and analyzing dynamical network models
https://neet.readthedocs.io/en/stable
Other
4 stars 10 forks source link

WTNetwork.network_graph includes fake self-edges #193

Open dglmoore opened 4 years ago

dglmoore commented 4 years ago

Description

We can convert a WTNetwork into a LogicNetwork which, by default, only includes edges if they have a demonstrable effect on the state of target node. The WTNetwork uses the Network.neighbors method to determine which edges exist when converting the network into a networkx.DiGraph. The result is fake edges, particularly self-edges, in the networkx.DiGraph.

Neet Version: 1.0.0 Operating System: ChromeOS 79

To Reproduce

  1. Convert neet.boolean.examples.s_pombe into a logic network via Network.network_graph.
  2. Compare the DiGraphs for the network before and after conversion.
  3. Compare the incoming neighbors of the nodes as determined by Network.neighbors.
>>> import neet
>>> import networkx as nx
>>> from neet.boolean.examples import s_pombe
>>> from neet.boolean.conv import wt_to_logic

# Incoming edges identified by Network.neighbors
>>> [s_pombe.neighbors(i, "in") for i in range(s_pombe.size)]
[{0}, {1, 2, 3, 4}, {0, 1, 2, 5, 8}, {0, 1, 3, 5, 8}, {4, 5}, {2, 3, 4, 5, 6, 7}, {8, 1, 6}, {8, 1, 7}, {8, 4}]
>>> [wt_to_logic(s_pombe).neighbors(i, "in") for i in range(s_pombe.size)]
[{0}, {2, 3, 4}, {0, 1, 2, 5, 8}, {0, 1, 3, 5, 8}, {5}, {2, 3, 4, 6, 7}, {8, 1, 6}, {8, 1, 7}, {4}]

# Incoming edges included in DiGraph generated by Network.network_graph
>>> [list(s_pombe.network_graph().predecessors(i)) for i in range(s_pombe.size)]
[[0], [1, 2, 3, 4], [0, 1, 2, 5, 8], [0, 1, 3, 5, 8], [4, 5], [2, 3, 4, 5, 6, 7], [1, 6, 8], [1, 7, 8], [4, 8]]
>>> [list(wt_to_logic(s_pombe).network_graph().predecessors(i)) for i in range(s_pombe.size)]
[[0], [2, 3, 4], [0, 1, 2, 5, 8], [0, 1, 3, 5, 8], [5], [2, 3, 4, 6, 7], [1, 6, 8], [1, 7, 8], [4]]

Expected Behavior

The Network.network_graph method should commute with neet.boolean.conv.wt_to_logic.

Actual Behavior

They don't commute.

hbsmith commented 4 years ago

Can I actually question the premise of this issue as a bug? It seems to me that because we don't try to "reduce" weight-threshold networks in the first place, that it makes sense that when you create a network_graph from the wt_network, you should expect that the neighbors from the wt network_graph match with the neighbors from the wt_network. Similarly, you'd expect that the neighbors for the logic_network match with the neighbors from the logic network_graph. This is the current behavior.

dglmoore commented 4 years ago

This is a fair point. However, say you want to keep the mean degree the same before and after randomization. Do you base that on the edges that have logical influence, or on the edges that have non-zero weights? This isn't usually an issue for edges between distinct nodes because most of the time the edges are real (even if they aren't, the associated weight is non-zero so we can assume), but it becomes one when you have a network with a WTNetwork.split_threshold function. In that case, we are introducing self edges regardless of whether or not they have logical consequence. It's reasonable, I think, to include a self edge in one of two cases:

  1. The appropriate diagonal element of the weight matrix is non-zero
  2. It exist by virtue of the threshold function.

Case 1 is trivial. Case 2 is less trivial (but still pretty easy to assess).

Ultimately, our randomization module will be built on the Network.network_graph method. Whatever graph it returns will be used for the purposes of topological randomization. We need to make sure the implementation of that method (and any overload of it) is bullet-proof.

hbsmith commented 4 years ago

Should we be "reducing" the WTNetwork when it is read-in then, instead of only doing it when the Network.network_graph method is called?

On Mon, Feb 3, 2020 at 3:01 PM Douglas G. Moore notifications@github.com wrote:

This is a fair point. However, say you want to keep the mean degree the same before and after randomization. Do you base that on the edges that have logical influence, or on the edges that have non-zero weights? This isn't usually an issue for edges between distinct nodes because most of the time the edges are real (even if they aren't, the associated weight is non-zero so we can assume), but it becomes one when you have a network with a WTNetwork.split_threshold function. In that case, we are introducing self edges regardless of whether or not they have logical consequence. It's reasonable, I think, to include a self edge in one of two cases:

  1. The appropriate diagonal element of the weight matrix is non-zero
  2. It exist by virtue of the threshold function.

Case 1 is trivial. Case 2 is less trivial (but still pretty easy to assess).

Ultimately, our randomization module will be built on the Network.network_graph method. Whatever graph it returns will be used for the purposes of topological randomization. We need to make sure the implementation of that method (and any overload of it) is bullet-proof.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ELIFE-ASU/Neet/issues/193?email_source=notifications&email_token=ACHE25FWUHHCSKY6J2KHLKDRA6XJXA5CNFSM4KMKS74KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKSTIFA#issuecomment-581252116, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACHE25GFMDBWJBYQ223NMQ3RA6XJXANCNFSM4KMKS74A .

dglmoore commented 4 years ago

I don't think so. I figure, if you include a false edge you deserve what you get. I'm really more worried about edges that are inadvertently introduced by us. The concern here is just the self-edges that are falsely introduced because we aren't careful with WTNetwork.split_threshold.