Issue 1126 (#1128)

* First attempt to fix. Rather experimental.

* Debugging and bug fixing

The basic issue was a missing break
However, correct computation of cache results for instance-to-instance cluster
interaction is implemented
Plus: identical and overlapping instances are no longer ignored except in the
case of exact duplicates. Otherwise these instance generate dead nets which
are not connected elsewhere.

* Added tests, fixed duplicate cells test, added missing files.

* Code simplification (removed invariant from transformation in cluster-to-cluster interaction cache)

* Skipping cell instance duplicates as some real-world testcases mandate so

* Updated test data

src/db/db/dbHierNetworkProcessor.cc

@@ -1582,8 +1582,7 @@ private:
    *  @param interacting_clusters_out Receives the cluster interaction descriptors
    *
    *  "interacting_clusters_out" will be cluster interactions in the parent instance space of i1 and i2 respectively.
-   *  Cluster ID will be valid in the parent cells containing i1 and i2 and the cluster instances property ID will be set to p1 and p2
-   *  respectively.
+   *  Cluster ID will be valid in the parent cells containing i1 and i2.
    */
   void consider_instance_pair (const box_type &common,
                                const db::Instance &i1, const db::ICplxTrans &t1, const db::CellInstArray::iterator &i1element,
@@ -1629,6 +1628,7 @@ private:
       db::ICplxTrans tt2 = t2 * i2t;
 
       db::ICplxTrans cache_norm = tt1.inverted ();
+
       ii_key = InstanceToInstanceInteraction ((! i1element.at_end () || i1.size () == 1) ? 0 : i1.cell_inst ().delegate (),
                                               (! i2element.at_end () || i2.size () == 1) ? 0 : i2.cell_inst ().delegate (),
                                               cache_norm, cache_norm * tt2);
@@ -1695,9 +1695,14 @@ private:
 
         db::ICplxTrans i2t = i2.complex_trans (*ii2);
         db::ICplxTrans tt2 = t2 * i2t;
+
         if (i1.cell_index () == i2.cell_index () && tt1 == tt2) {
           //  skip interactions between identical instances (duplicate instance removal)
-          continue;
+          if (! i2element.at_end ()) {
+            break;
+          } else {
+            continue;
+          }
         }
 
         box_type ib2 = bb2.transformed (tt2);
@@ -1903,6 +1908,7 @@ private:
             for (std::list<std::pair<ClusterInstance, ClusterInstance> >::iterator ii = ii_interactions.begin (); ii != ii_interactions.end (); ++ii) {
               propagate_cluster_inst (ii->second, i.cell_index (), tt2, i.prop_id ());
             }
+
             interacting_clusters.splice (interacting_clusters.end (), ii_interactions, ii_interactions.begin (), ii_interactions.end ());
 
           }
@@ -1974,6 +1980,7 @@ private:
           for (typename std::list<ClusterInstanceInteraction>::iterator ii = ci_interactions.begin (); ii != ci_interactions.end (); ++ii) {
             propagate_cluster_inst (ii->other_ci, i2.cell_index (), i2t, i2.prop_id ());
           }
+
           interactions_out.splice (interactions_out.end (), ci_interactions, ci_interactions.begin (), ci_interactions.end ());
 
         }
@@ -2088,6 +2095,10 @@ private:
    *
    *  After calling this method, the cluster instance in ci is guaranteed to have connections from all
    *  parent cells. One of these connections represents the instance ci.
+   *
+   *  Returns false if the connection was already there in the same place (indicating duplicate instances).
+   *  In this case, the cluster instance should be skipped. In the other case, the cluster instance is
+   *  updated to reflect the connected cluster.
    */
   void propagate_cluster_inst (ClusterInstance &ci, db::cell_index_type pci, const db::ICplxTrans &trans, db::properties_id_type prop_id) const
   {
@@ -2178,7 +2189,7 @@ hier_clusters<T>::propagate_cluster_inst (const db::Layout &layout, const db::Ce
 
   if (parent_cluster > 0) {
 
-    //  taken parent
+    //  take parent
     id_new = parent_cluster;
 
   } else {

src/db/unit_tests/dbHierNetworkProcessorTests.cc

@@ -1353,3 +1353,48 @@ TEST(200_issue609)
     EXPECT_EQ (root_nets (hc.clusters_per_cell (*td)), size_t (0));
   }
 }
+
+//  issue #1126
+TEST(201_issue1126)
+{
+  {
+    db::Layout ly;
+    unsigned int l1 = 0;
+
+    {
+      db::LayerProperties p;
+      db::LayerMap lmap;
+
+      p.layer = 1;
+      p.datatype = 0;
+      lmap.map (db::LDPair (p.layer, p.datatype), l1 = ly.insert_layer ());
+      ly.set_properties (l1, p);
+
+      db::LoadLayoutOptions options;
+      options.get_options<db::CommonReaderOptions> ().layer_map = lmap;
+      options.get_options<db::CommonReaderOptions> ().create_other_layers = false;
+
+      std::string fn (tl::testdata ());
+      fn += "/algo/issue-1126.gds.gz";
+      tl::InputStream stream (fn);
+      db::Reader reader (stream);
+      reader.read (ly, options);
+    }
+
+    std::vector<std::string> strings;
+    normalize_layer (ly, strings, l1);
+
+    //  connect 1 to 1
+    db::Connectivity conn;
+    conn.connect (l1, l1);
+
+    db::hier_clusters<db::PolygonRef> hc;
+    hc.build (ly, ly.cell (*ly.begin_top_down ()), conn);
+
+    // should not assert until here
+  }
+
+  //  detailed test:
+  run_hc_test (_this, "issue-1126.gds.gz", "issue-1126_au.gds");
+}
+

src/db/unit_tests/dbLayoutToNetlistTests.cc

@@ -2846,6 +2846,7 @@ TEST(11_DuplicateInstances)
   //  compare netlist as string
   CHECKPOINT ();
   db::compare_netlist (_this, *l2n.netlist (),
+    //  suppressing duplicate cells:
     "circuit RINGO ();\n"
     "  subcircuit INV2 $1 (IN=$I12,$2=FB,OUT=OSC,$4=VSS,$5=VDD);\n"
     "  subcircuit INV2 $2 (IN=FB,$2=$I25,OUT=$I1,$4=VSS,$5=VDD);\n"