Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WTNetwork.network_graph includes fake self-edges #193

Open
dglmoore opened this issue Jan 28, 2020 · 4 comments
Open

WTNetwork.network_graph includes fake self-edges #193

dglmoore opened this issue Jan 28, 2020 · 4 comments
Labels

Comments

@dglmoore
Copy link
Contributor

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.

@dglmoore dglmoore added the bug label Jan 28, 2020
@dglmoore dglmoore mentioned this issue Jan 28, 2020
15 tasks
@hbsmith
Copy link
Contributor

hbsmith commented Jan 29, 2020

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
Copy link
Contributor Author

dglmoore commented Feb 3, 2020

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
Copy link
Contributor

hbsmith commented Feb 3, 2020 via email

@dglmoore
Copy link
Contributor Author

dglmoore commented Feb 3, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants