[ENH, BUG]: added colliders and v_structures and deprecated `comp…

…ute_v_structures` in `dag.py` (networkx#7398)

Deprecates `compute_v_structures` in favor of two newly added
generators in the `dag` module: `colliders` and `v_structures`

Resolves an issue with `compute_v_structures` which incorrectly included all
colliders, as opposed to only the subset of colliders with no edge between the
parents.

This PR fixes that bug, improves the terminology/documentation, and improves
the interface via deprecating the buggy compute_v_structures.

---------

Co-authored-by: Dan Schult <[email protected]>
Co-authored-by: Ross Barnowski <[email protected]>

doc/developer/deprecations.rst

@@ -68,3 +68,7 @@ Version 3.5
 * Remove ``total_spanning_tree_weight`` from ``linalg/laplacianmatrix.py``
 * Remove ``create`` keyword argument from ``nonisomorphic_trees`` in 
   ``generators/nonisomorphic_trees``.
+
+Version 3.6
+~~~~~~~~~~~
+* Remove ``compute_v_structures`` from ``algorithms/dag.py``.

doc/reference/algorithms/dag.rst

@@ -22,3 +22,5 @@ Directed Acyclic Graphs
    dag_longest_path_length
    dag_to_branching
    compute_v_structures
+   colliders
+   v_structures

networkx/algorithms/dag.py

@@ -1224,36 +1224,191 @@ def dag_to_branching(G):
 @not_implemented_for("undirected")
 @nx._dispatchable
 def compute_v_structures(G):
-    """Iterate through the graph to compute all v-structures.
+    """Yields 3-node tuples that represent the v-structures in `G`.
 
-    V-structures are triples in the directed graph where
-    two parent nodes point to the same child and the two parent nodes
-    are not adjacent.
+    .. deprecated:: 3.4
+
+       `compute_v_structures` actually yields colliders. It will be removed in
+       version 3.6. Use `nx.dag.v_structures` or `nx.dag.colliders` instead.
+
+    Colliders are triples in the directed acyclic graph (DAG) where two parent nodes
+    point to the same child node. V-structures are colliders where the two parent
+    nodes are not adjacent. In a causal graph setting, the parents do not directly
+    depend on each other, but conditioning on the child node provides an association.
 
     Parameters
     ----------
     G : graph
-        A networkx DiGraph.
+        A networkx `~networkx.DiGraph`.
 
-    Returns
-    -------
-    vstructs : iterator of tuples
-        The v structures within the graph. Each v structure is a 3-tuple with the
-        parent, collider, and other parent.
+    Yields
+    ------
+    A 3-tuple representation of a v-structure
+        Each v-structure is a 3-tuple with the parent, collider, and other parent.
+
+    Raises
+    ------
+    NetworkXNotImplemented
+        If `G` is an undirected graph.
 
     Examples
     --------
-    >>> G = nx.DiGraph()
-    >>> G.add_edges_from([(1, 2), (0, 5), (3, 1), (2, 4), (3, 1), (4, 5), (1, 5)])
-    >>> sorted(nx.compute_v_structures(G))
-    [(0, 5, 1), (0, 5, 4), (1, 5, 4)]
+    >>> G = nx.DiGraph([(1, 2), (0, 4), (3, 1), (2, 4), (0, 5), (4, 5), (1, 5)])
+    >>> nx.is_directed_acyclic_graph(G)
+    True
+    >>> list(nx.compute_v_structures(G))
+    [(0, 4, 2), (0, 5, 4), (0, 5, 1), (4, 5, 1)]
+
+    See Also
+    --------
+    v_structures
+    colliders
 
     Notes
     -----
-    `Wikipedia: Collider in causal graphs <https://en.wikipedia.org/wiki/Collider_(statistics)>`_
+    This function was written to be used on DAGs, however it works on cyclic graphs
+    too. Since colliders are referred to in the cyclic causal graph literature
+    [2]_ we allow cyclic graphs in this function. It is suggested that you test if
+    your input graph is acyclic as in the example if you want that property.
+
+    References
+    ----------
+    .. [1]  `Pearl's PRIMER <https://bayes.cs.ucla.edu/PRIMER/primer-ch2.pdf>`_
+            Ch-2 page 50: v-structures def.
+    .. [2] A Hyttinen, P.O. Hoyer, F. Eberhardt, M J ̈arvisalo, (2013)
+           "Discovering cyclic causal models with latent variables:
+           a general SAT-based procedure", UAI'13: Proceedings of the Twenty-Ninth
+           Conference on Uncertainty in Artificial Intelligence, pg 301–310,
+           `doi:10.5555/3023638.3023669 <https://dl.acm.org/doi/10.5555/3023638.3023669>`_
+    """
+    import warnings
+
+    warnings.warn(
+        (
+            "\n\n`compute_v_structures` actually yields colliders. It will be\n"
+            "removed in version 3.6. Use `nx.dag.v_structures` or `nx.dag.colliders`\n"
+            "instead.\n"
+        ),
+        category=DeprecationWarning,
+        stacklevel=5,
+    )
+
+    return colliders(G)
+
+
+@not_implemented_for("undirected")
+@nx._dispatchable
+def v_structures(G):
+    """Yields 3-node tuples that represent the v-structures in `G`.
+
+    Colliders are triples in the directed acyclic graph (DAG) where two parent nodes
+    point to the same child node. V-structures are colliders where the two parent
+    nodes are not adjacent. In a causal graph setting, the parents do not directly
+    depend on each other, but conditioning on the child node provides an association.
+
+    Parameters
+    ----------
+    G : graph
+        A networkx `~networkx.DiGraph`.
+
+    Yields
+    ------
+    A 3-tuple representation of a v-structure
+        Each v-structure is a 3-tuple with the parent, collider, and other parent.
+
+    Raises
+    ------
+    NetworkXNotImplemented
+        If `G` is an undirected graph.
+
+    Examples
+    --------
+    >>> G = nx.DiGraph([(1, 2), (0, 4), (3, 1), (2, 4), (0, 5), (4, 5), (1, 5)])
+    >>> nx.is_directed_acyclic_graph(G)
+    True
+    >>> list(nx.dag.v_structures(G))
+    [(0, 4, 2), (0, 5, 1), (4, 5, 1)]
+
+    See Also
+    --------
+    colliders
+
+    Notes
+    -----
+    This function was written to be used on DAGs, however it works on cyclic graphs
+    too. Since colliders are referred to in the cyclic causal graph literature
+    [2]_ we allow cyclic graphs in this function. It is suggested that you test if
+    your input graph is acyclic as in the example if you want that property.
+
+    References
+    ----------
+    .. [1]  `Pearl's PRIMER <https://bayes.cs.ucla.edu/PRIMER/primer-ch2.pdf>`_
+            Ch-2 page 50: v-structures def.
+    .. [2] A Hyttinen, P.O. Hoyer, F. Eberhardt, M J ̈arvisalo, (2013)
+           "Discovering cyclic causal models with latent variables:
+           a general SAT-based procedure", UAI'13: Proceedings of the Twenty-Ninth
+           Conference on Uncertainty in Artificial Intelligence, pg 301–310,
+           `doi:10.5555/3023638.3023669 <https://dl.acm.org/doi/10.5555/3023638.3023669>`_
+    """
+    for p1, c, p2 in colliders(G):
+        if not (G.has_edge(p1, p2) or G.has_edge(p2, p1)):
+            yield (p1, c, p2)
+
+
+@not_implemented_for("undirected")
+@nx._dispatchable
+def colliders(G):
+    """Yields 3-node tuples that represent the colliders in `G`.
+
+    In a Directed Acyclic Graph (DAG), if you have three nodes A, B, and C, and
+    there are edges from A to C and from B to C, then C is a collider [1]_ . In
+    a causal graph setting, this means that both events A and B are "causing" C,
+    and conditioning on C provide an association between A and B even if
+    no direct causal relationship exists between A and B.
+
+    Parameters
+    ----------
+    G : graph
+        A networkx `~networkx.DiGraph`.
+
+    Yields
+    ------
+    A 3-tuple representation of a collider
+        Each collider is a 3-tuple with the parent, collider, and other parent.
+
+    Raises
+    ------
+    NetworkXNotImplemented
+        If `G` is an undirected graph.
+
+    Examples
+    --------
+    >>> G = nx.DiGraph([(1, 2), (0, 4), (3, 1), (2, 4), (0, 5), (4, 5), (1, 5)])
+    >>> nx.is_directed_acyclic_graph(G)
+    True
+    >>> list(nx.dag.colliders(G))
+    [(0, 4, 2), (0, 5, 4), (0, 5, 1), (4, 5, 1)]
+
+    See Also
+    --------
+    v_structures
+
+    Notes
+    -----
+    This function was written to be used on DAGs, however it works on cyclic graphs
+    too. Since colliders are referred to in the cyclic causal graph literature
+    [2]_ we allow cyclic graphs in this function. It is suggested that you test if
+    your input graph is acyclic as in the example if you want that property.
+
+    References
+    ----------
+    .. [1] `Wikipedia: Collider in causal graphs <https://en.wikipedia.org/wiki/Collider_(statistics)>`_
+    .. [2] A Hyttinen, P.O. Hoyer, F. Eberhardt, M J ̈arvisalo, (2013)
+           "Discovering cyclic causal models with latent variables:
+           a general SAT-based procedure", UAI'13: Proceedings of the Twenty-Ninth
+           Conference on Uncertainty in Artificial Intelligence, pg 301–310,
+           `doi:10.5555/3023638.3023669 <https://dl.acm.org/doi/10.5555/3023638.3023669>`_
     """
-    for collider, preds in G.pred.items():
-        for common_parents in combinations(preds, r=2):
-            # ensure that the colliders are the same
-            common_parents = sorted(common_parents)
-            yield (common_parents[0], collider, common_parents[1])
+    for node in G.nodes:
+        for p1, p2 in combinations(G.predecessors(node), 2):
+            yield (p1, node, p2)

networkx/algorithms/tests/test_dag.py

@@ -760,7 +760,8 @@ def test_ancestors_descendants_undirected():
 
 def test_compute_v_structures_raise():
     G = nx.Graph()
-    pytest.raises(nx.NetworkXNotImplemented, nx.compute_v_structures, G)
+    with pytest.raises(nx.NetworkXNotImplemented, match="for undirected type"):
+        nx.compute_v_structures(G)
 
 
 def test_compute_v_structures():
@@ -775,3 +776,53 @@ def test_compute_v_structures():
     G = nx.DiGraph(edges)
     v_structs = set(nx.compute_v_structures(G))
     assert len(v_structs) == 2
+
+
+def test_compute_v_structures_deprecated():
+    G = nx.DiGraph()
+    with pytest.deprecated_call():
+        nx.compute_v_structures(G)
+
+
+def test_v_structures_raise():
+    G = nx.Graph()
+    with pytest.raises(nx.NetworkXNotImplemented, match="for undirected type"):
+        nx.dag.v_structures(G)
+
+
+def test_v_structures():
+    edges = [(0, 1), (0, 2), (3, 2)]
+    G = nx.DiGraph(edges)
+
+    v_structs = set(nx.dag.v_structures(G))
+    assert len(v_structs) == 1
+    assert (0, 2, 3) in v_structs
+
+    edges = [("A", "B"), ("C", "B"), ("D", "G"), ("D", "E"), ("G", "E")]
+    G = nx.DiGraph(edges)
+    v_structs = set(nx.dag.v_structures(G))
+    assert v_structs == {("A", "B", "C")}
+
+    edges = [(0, 1), (2, 1), (0, 2)]  # adjacent parents case: issues#7385
+    G = nx.DiGraph(edges)
+    assert set(nx.dag.v_structures(G)) == set()
+
+
+def test_colliders_raise():
+    G = nx.Graph()
+    with pytest.raises(nx.NetworkXNotImplemented, match="for undirected type"):
+        nx.dag.colliders(G)
+
+
+def test_colliders():
+    edges = [(0, 1), (0, 2), (3, 2)]
+    G = nx.DiGraph(edges)
+
+    colliders = set(nx.dag.colliders(G))
+    assert len(colliders) == 1
+    assert (0, 2, 3) in colliders
+
+    edges = [("A", "B"), ("C", "B"), ("D", "G"), ("D", "E"), ("G", "E")]
+    G = nx.DiGraph(edges)
+    colliders = set(nx.dag.colliders(G))
+    assert colliders == {("A", "B", "C"), ("D", "E", "G")}

networkx/conftest.py

@@ -151,6 +151,9 @@ def set_warnings():
     warnings.filterwarnings(
         "ignore", category=DeprecationWarning, message=r"\n\nThe 'create=matrix'"
     )
+    warnings.filterwarnings(
+        "ignore", category=DeprecationWarning, message="\n\ncompute_v_structures"
+    )
 
 
 @pytest.fixture(autouse=True)