Fix VF2 in case of empty graphs. (Qiskit#423)

* Fix VF2 in case of empty graphs.

An alternative for Qiskit#421. This commit fixes the output of functions
solving the isomorphism problem (`is_isomorphic`, `is_subgraph_isomorphic`,
`vf2_mapping`) without handling empty graphs as a special case but rather
they now fall naturally into the logic of the algorithm. The solution is to
equivalently check if we have found a complete mapping in the `Outer` phase
and not in the `Inner` phase of the algorithm, where we dont reach in case
of an empty graph.

* release note

* irrelevant code simplification

releasenotes/notes/fix-vf2-mapping-empty-graph-1ef7a1aef9d84758.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Previously, :func:`~retworkx.vf2_mapping` function when
+    comparing 2 empty graph objects would incorrectly return an
+    empty iterator. This has been fixed and now returns an iterator
+    over the single valid mapping, i.e the empty mapping.

src/isomorphism/vf2.rs

@@ -16,7 +16,7 @@
 // since diverged significantly from the original petgraph implementation.
 
 use fixedbitset::FixedBitSet;
-use std::cmp::Ordering;
+use std::cmp::{Ordering, Reverse};
 use std::iter::Iterator;
 use std::marker;
 
@@ -133,7 +133,7 @@ where
                             dout[node],
                             conn_out[node],
                             din[node],
-                            -(node as isize),
+                            Reverse(node),
                         )
                     })
                     .unwrap();
@@ -193,7 +193,7 @@ where
         let mut sorted_nodes: Vec<usize> =
             graph.node_indices().map(|node| node.index()).collect();
         sorted_nodes
-            .par_sort_by_key(|&node| (dout[node], din[node], -(node as isize)));
+            .par_sort_by_key(|&node| (dout[node], din[node], Reverse(node)));
         sorted_nodes.reverse();
 
         for node in sorted_nodes {
@@ -375,12 +375,6 @@ pub fn is_isomorphic<Ty: EdgeType>(
     {
         return Ok(false);
     }
-    // TODO: Remove this. This is just a hacky workaround to fix #421 fast,
-    // we should fix VF2Algorithm.next() to return an empty hashmap for
-    // 2 empty graphs
-    if g1.node_count() == 0 && g1.edge_count() == 0 {
-        return Ok(true);
-    }
 
     let mut vf2 = Vf2Algorithm::new(
         py, g0, g1, node_match, edge_match, id_order, ordering, induced,
@@ -845,7 +839,12 @@ where
                 Frame::Outer => {
                     match Vf2Algorithm::<Ty, F, G>::next_candidate(&mut self.st)
                     {
-                        None => continue,
+                        None => {
+                            if self.st[1].is_complete() {
+                                return Ok(Some(self.mapping()));
+                            }
+                            continue;
+                        }
                         Some((nx, mx, ol)) => {
                             let f = Frame::Inner {
                                 nodes: [nx, mx],
@@ -872,20 +871,6 @@ where
                             &mut self.st,
                             nodes,
                         );
-                        if self.st[1].is_complete() {
-                            let f0 = Frame::Unwind {
-                                nodes,
-                                open_list: ol,
-                            };
-                            self.stack.push(f0);
-                            return Ok(Some(self.mapping()));
-                        }
-                        self._counter += 1;
-                        if let Some(limit) = self.call_limit {
-                            if self._counter > limit {
-                                return Ok(None);
-                            }
-                        }
                         // Check cardinalities of Tin, Tout sets
                         if self.st[0]
                             .out_size
@@ -898,6 +883,12 @@ where
                                 .then(self.ordering)
                                 == self.ordering
                         {
+                            self._counter += 1;
+                            if let Some(limit) = self.call_limit {
+                                if self._counter > limit {
+                                    return Ok(None);
+                                }
+                            }
                             let f0 = Frame::Unwind {
                                 nodes,
                                 open_list: ol,

tests/digraph/test_isomorphic.py

@@ -17,7 +17,7 @@
 
 
 class TestIsomorphic(unittest.TestCase):
-    def test_empty_isomorphic_identical(self):
+    def test_empty_isomorphic(self):
         dag_a = retworkx.PyDAG()
         dag_b = retworkx.PyDAG()
 
@@ -27,17 +27,7 @@ def test_empty_isomorphic_identical(self):
                     retworkx.is_isomorphic(dag_a, dag_b, id_order=id_order)
                 )
 
-    def test_empty_isomorphic_mismatch_node_data(self):
-        dag_a = retworkx.PyDAG()
-        dag_b = retworkx.PyDAG()
-
-        for id_order in [False, True]:
-            with self.subTest(id_order=id_order):
-                self.assertTrue(
-                    retworkx.is_isomorphic(dag_a, dag_b, id_order=id_order)
-                )
-
-    def test_empty_isomorphic_compare_nodes_mismatch_node_data(self):
+    def test_empty_isomorphic_compare_nodes(self):
         dag_a = retworkx.PyDAG()
         dag_b = retworkx.PyDAG()
 
@@ -368,3 +358,13 @@ def test_digraph_vf2_number_of_valid_mappings(self):
         for _ in mapping:
             total += 1
         self.assertEqual(total, 6)
+
+    def test_empty_digraph_vf2_mapping(self):
+        g_a = retworkx.PyDiGraph()
+        g_b = retworkx.PyDiGraph()
+        for id_order in [False, True]:
+            with self.subTest(id_order=id_order):
+                mapping = retworkx.digraph_vf2_mapping(
+                    g_a, g_b, id_order=id_order, subgraph=False
+                )
+                self.assertEqual({}, next(mapping))

tests/digraph/test_subgraph_isomorphic.py

@@ -24,7 +24,7 @@ def test_empty_subgraph_isomorphic_identical(self):
                     retworkx.is_subgraph_isomorphic(g_a, g_a, id_order=id_order)
                 )
 
-    def test_empty_subgraph_isomorphic_mismatch_node_data(self):
+    def test_empty_subgraph_isomorphic(self):
         g_a = retworkx.PyDiGraph()
         g_b = retworkx.PyDiGraph()
         for id_order in [False, True]:
@@ -33,7 +33,7 @@ def test_empty_subgraph_isomorphic_mismatch_node_data(self):
                     retworkx.is_subgraph_isomorphic(g_a, g_b, id_order=id_order)
                 )
 
-    def test_empty_subgraph_isomorphic_compare_nodes_mismatch_node_data(self):
+    def test_empty_subgraph_isomorphic_compare_nodes(self):
         g_a = retworkx.PyDiGraph()
         g_b = retworkx.PyDiGraph()
         for id_order in [False, True]:
@@ -272,3 +272,13 @@ def test_vf2pp_remapping(self):
             graph, second_graph, subgraph=True, id_order=False
         )
         self.assertEqual(next(mapping), {5: 0, 6: 1, 8: 2, 9: 3})
+
+    def test_empty_digraph_subgraph_vf2_mapping(self):
+        g_a = retworkx.PyDiGraph()
+        g_b = retworkx.PyDiGraph()
+        for id_order in [False, True]:
+            with self.subTest(id_order=id_order):
+                mapping = retworkx.digraph_vf2_mapping(
+                    g_a, g_b, id_order=id_order, subgraph=True
+                )
+                self.assertEqual({}, next(mapping))

tests/graph/test_isomorphic.py

@@ -25,7 +25,7 @@ def test_empty_isomorphic_identical(self):
                     retworkx.is_isomorphic(g_a, g_b, id_order=id_order)
                 )
 
-    def test_empty_isomorphic_mismatch_node_data(self):
+    def test_empty_isomorphic(self):
         g_a = retworkx.PyGraph()
         g_b = retworkx.PyGraph()
         for id_order in [False, True]:
@@ -34,7 +34,7 @@ def test_empty_isomorphic_mismatch_node_data(self):
                     retworkx.is_isomorphic(g_a, g_b, id_order=id_order)
                 )
 
-    def test_empty_isomorphic_compare_nodes_mismatch_node_data(self):
+    def test_empty_isomorphic_compare_nodes(self):
         g_a = retworkx.PyGraph()
         g_b = retworkx.PyGraph()
         for id_order in [False, True]:
@@ -328,3 +328,13 @@ def test_graph_vf2_number_of_valid_mappings(self):
         for _ in mapping:
             total += 1
         self.assertEqual(total, 6)
+
+    def test_empty_graph_vf2_mapping(self):
+        g_a = retworkx.PyGraph()
+        g_b = retworkx.PyGraph()
+        for id_order in [False, True]:
+            with self.subTest(id_order=id_order):
+                mapping = retworkx.graph_vf2_mapping(
+                    g_a, g_b, id_order=id_order, subgraph=False
+                )
+                self.assertEqual({}, next(mapping))

tests/graph/test_subgraph_isomorphic.py

@@ -24,7 +24,7 @@ def test_empty_subgraph_isomorphic_identical(self):
                     retworkx.is_subgraph_isomorphic(g_a, g_a, id_order=id_order)
                 )
 
-    def test_empty_subgraph_isomorphic_mismatch_node_data(self):
+    def test_empty_subgraph_isomorphic(self):
         g_a = retworkx.PyGraph()
         g_b = retworkx.PyGraph()
         for id_order in [False, True]:
@@ -33,7 +33,7 @@ def test_empty_subgraph_isomorphic_mismatch_node_data(self):
                     retworkx.is_subgraph_isomorphic(g_a, g_b, id_order=id_order)
                 )
 
-    def test_empty_subgraph_isomorphic_compare_nodes_mismatch_node_data(self):
+    def test_empty_subgraph_isomorphic_compare_nodes(self):
         g_a = retworkx.PyGraph()
         g_b = retworkx.PyGraph()
         for id_order in [False, True]:
@@ -272,3 +272,25 @@ def test_vf2pp_remapping(self):
             graph, second_graph, subgraph=True, id_order=False
         )
         self.assertEqual(next(mapping), {5: 0, 4: 2, 1: 3, 2: 1})
+
+    def test_empty_subgraph_vf2_mapping(self):
+        g_a = retworkx.PyGraph()
+        g_b = retworkx.PyGraph()
+        for id_order in [False, True]:
+            with self.subTest(id_order=id_order):
+                mapping = retworkx.graph_vf2_mapping(
+                    g_a, g_b, id_order=id_order, subgraph=True
+                )
+                self.assertEqual({}, next(mapping))
+
+    def test_subgraph_vf2_mapping_out_size(self):
+        first = retworkx.PyGraph()
+        first.add_nodes_from([0, 1, 2, 3])
+        first.add_edges_from_no_data([(0, 1), (0, 2), (1, 2), (2, 3)])
+        second = retworkx.PyGraph()
+        second.add_nodes_from([0, 1, 2, 3])
+        second.add_edges_from_no_data([(0, 1), (0, 2), (1, 3)])
+        mapping = retworkx.graph_vf2_mapping(
+            first, second, subgraph=True, id_order=True, induced=False
+        )
+        self.assertEqual(next(mapping), {0: 0, 1: 2, 2: 1, 3: 3})