Fix problems with subcircuit finding (#1292)

pytket/conanfile.py

@@ -32,7 +32,7 @@ def package(self):
         cmake.install()
 
     def requirements(self):
-        self.requires("tket/1.2.100@tket/stable")
+        self.requires("tket/1.2.101@tket/stable")
         self.requires("tklog/0.3.3@tket/stable")
         self.requires("tkrng/0.3.3@tket/stable")
         self.requires("tkassert/0.3.4@tket/stable")

tket/conanfile.py

@@ -23,7 +23,7 @@
 
 class TketConan(ConanFile):
     name = "tket"
-    version = "1.2.100"
+    version = "1.2.101"
     package_type = "library"
     license = "Apache 2"
     homepage = "https://github.com/CQCL/tket"

tket/include/tket/Circuit/DAGDefs.hpp

@@ -14,6 +14,7 @@
 
 #pragma once
 
+#include <cstddef>
 #include <list>
 #include <optional>
 #include <set>
@@ -74,7 +75,7 @@ typedef boost::graph_traits<DAG>::vertex_iterator V_iterator;
 typedef std::unordered_set<Vertex> VertexSet;
 typedef std::vector<Vertex> VertexVec;
 typedef std::list<Vertex> VertexList;
-typedef std::unordered_map<Vertex, unsigned> IndexMap;
+typedef std::unordered_map<Vertex, std::size_t> IndexMap;
 typedef boost::adj_list_vertex_property_map<
     DAG, std::size_t, std::size_t&, boost::vertex_index_t>
     VIndex;
@@ -85,7 +86,7 @@ typedef boost::adj_list_vertex_property_map<
  * This can be used instead of a plain @ref Vertex in associative containers
  * where control over the order of iteration is required.
  */
-typedef std::pair<unsigned, Vertex> IVertex;
+typedef std::pair<std::size_t, Vertex> IVertex;
 
 typedef boost::graph_traits<DAG>::edge_descriptor Edge;
 typedef boost::graph_traits<DAG>::edge_iterator E_iterator;

tket/src/Circuit/Circuit.cpp

@@ -134,7 +134,7 @@ VertexVec Circuit::vertices_in_order() /*const*/ {
 
 IndexMap Circuit::index_map() const {
   IndexMap im;
-  unsigned i = 0;
+  std::size_t i = 0;
   BGL_FORALL_VERTICES(v, dag, DAG) { im[v] = i++; }
   return im;
 }

tket/src/Circuit/SubcircuitFinder.cpp

@@ -55,6 +55,16 @@ static bool union_is_connected(
   return false;
 }
 
+static VertexSet set_diff(const VertexSet &A, const VertexSet &B) {
+  VertexSet C;
+  for (const Vertex &v : A) {
+    if (!B.contains(v)) {
+      C.insert(v);
+    }
+  }
+  return C;
+}
+
 // Return the union of two disjoint convex connected subcircuits, assuming that
 // the union is convex and connected.
 static subcircuit_info_t convex_union(
@@ -71,37 +81,22 @@ static subcircuit_info_t convex_union(
   VertexSet verts = verts0;
   verts.insert(verts1.begin(), verts1.end());
 
-  // preds = (preds0 ∖ verts1) ∪ (preds1 ∖ verts0)
-  VertexVec p0mv1;
-  std::set_difference(
-      preds0.begin(), preds0.end(), verts1.begin(), verts1.end(),
-      std::inserter(p0mv1, p0mv1.begin()));
-  VertexVec p1mv0;
-  std::set_difference(
-      preds1.begin(), preds1.end(), verts0.begin(), verts0.end(),
-      std::inserter(p1mv0, p1mv0.begin()));
-  VertexSet preds{p0mv1.begin(), p0mv1.end()};
-  preds.insert(p1mv0.begin(), p1mv0.end());
+  // preds = (preds0 ∪ preds1) ∖ verts
+  VertexSet preds01 = preds0;
+  preds01.insert(preds1.begin(), preds1.end());
+  VertexSet preds = set_diff(preds01, verts);
 
-  // succs = (succs0 ∖ verts1) ∪ (succs1 ∖ verts0)
-  VertexVec s0mv1;
-  std::set_difference(
-      succs0.begin(), succs0.end(), verts1.begin(), verts1.end(),
-      std::inserter(s0mv1, s0mv1.begin()));
-  VertexVec s1mv0;
-  std::set_difference(
-      succs1.begin(), succs1.end(), verts0.begin(), verts0.end(),
-      std::inserter(s1mv0, s1mv0.begin()));
-  VertexSet succs{s0mv1.begin(), s0mv1.end()};
-  succs.insert(s1mv0.begin(), s1mv0.end());
+  // succs = (succs0 ∪ succs1) ∖ verts
+  VertexSet succs01 = succs0;
+  succs01.insert(succs1.begin(), succs1.end());
+  VertexSet succs = set_diff(succs01, verts);
 
   return {verts, preds, succs};
 }
 
 static std::set<std::pair<Vertex, Vertex>> order_relations(
     Circuit *circ /*const*/) {
   // Put the vertices in reverse topological order:
-  circ->index_vertices();
   VertexVec verts;
   boost::topological_sort(circ->dag, std::back_inserter(verts));
   // Construct a map v --> {all vertices in the future of v}:
@@ -181,7 +176,7 @@ class SubcircuitFinder {
     const VertexSet &preds0 = subcircuit_info0.preds;
     const VertexSet &succs0 = subcircuit_info0.succs;
     const VertexSet &preds1 = subcircuit_info1.preds;
-    const VertexSet &succs1 = subcircuit_info0.succs;
+    const VertexSet &succs1 = subcircuit_info1.succs;
     for (const Vertex &v0 : succs0) {
       for (const Vertex &v1 : preds1) {
         if (order_relations_.contains({v0, v1})) {
@@ -223,6 +218,7 @@ class SubcircuitFinder {
 
 std::vector<VertexSet> Circuit::get_subcircuits(
     std::function<bool(Op_ptr)> criterion) /*const*/ {
+  index_vertices();
   SubcircuitFinder finder(this);
   return finder.find_subcircuits(criterion);
 }

tket/test/src/Passes/test_CliffordResynthesis.cpp

@@ -92,7 +92,7 @@ SCENARIO("SynthesiseCliffordResynthesis correctness") {
     c.add_op<unsigned>(OpType::ZZMax, {0, 1});
     check_clifford_resynthesis(c);
   }
-  GIVEN("A troublesome circuit") {
+  GIVEN("A troublesome circuit (1)") {
     // https://github.com/CQCL/tket/issues/1279
     Circuit c(3);
     c.add_op<unsigned>(OpType::ECR, {1, 2});
@@ -101,6 +101,14 @@ SCENARIO("SynthesiseCliffordResynthesis correctness") {
     c.add_op<unsigned>(OpType::ZZMax, {2, 1});
     check_clifford_resynthesis(c);
   }
+  GIVEN("A troublesome circuit (2)") {
+    Circuit c(2);
+    c.add_op<unsigned>(OpType::CX, {0, 1});
+    c.add_op<unsigned>(OpType::Z, {0});
+    c.add_op<unsigned>(OpType::T, {0});
+    c.add_op<unsigned>(OpType::CY, {1, 0});
+    check_clifford_resynthesis(c);
+  }
 }
 
 }  // namespace test_CliffordResynthesis