From 20099a38b939bfc96d53c3592a3bf31c615f72c4 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 7 Sep 2024 22:39:20 +0200 Subject: [PATCH 1/3] Fixing second part - small L,R and C were not handled properly --- src/db/db/dbNetlistDeviceClasses.cc | 6 +- src/db/unit_tests/dbNetlistTests.cc | 196 ++++++++++++++++++++++++++++ 2 files changed, 199 insertions(+), 3 deletions(-) diff --git a/src/db/db/dbNetlistDeviceClasses.cc b/src/db/db/dbNetlistDeviceClasses.cc index 2a1fa95736..5252557d0b 100644 --- a/src/db/db/dbNetlistDeviceClasses.cc +++ b/src/db/db/dbNetlistDeviceClasses.cc @@ -116,7 +116,7 @@ class ResistorDeviceCombiner { double va = a->parameter_value (0); double vb = b->parameter_value (0); - a->set_parameter_value (0, va + vb < 1e-10 ? 0.0 : va * vb / (va + vb)); + a->set_parameter_value (0, va + vb < 1e-30 ? 0.0 : va * vb / (va + vb)); // parallel width is sum of both, length is the one that gives the same value of resistance // R = 1/(1/R1 + 1/R2) @@ -204,7 +204,7 @@ class CapacitorDeviceCombiner { double va = a->parameter_value (0); double vb = b->parameter_value (0); - a->set_parameter_value (0, va + vb < 1e-10 ? 0.0 : va * vb / (va + vb)); + a->set_parameter_value (0, va + vb < 1e-30 ? 0.0 : va * vb / (va + vb)); // TODO: does this implementation make sense? double aa = a->parameter_value (1); @@ -259,7 +259,7 @@ class InductorDeviceCombiner { double va = a->parameter_value (0); double vb = b->parameter_value (0); - a->set_parameter_value (0, va + vb < 1e-10 ? 0.0 : va * vb / (va + vb)); + a->set_parameter_value (0, va + vb < 1e-30 ? 0.0 : va * vb / (va + vb)); } void serial (Device *a, Device *b) const diff --git a/src/db/unit_tests/dbNetlistTests.cc b/src/db/unit_tests/dbNetlistTests.cc index 6cb051c86d..685342a3eb 100644 --- a/src/db/unit_tests/dbNetlistTests.cc +++ b/src/db/unit_tests/dbNetlistTests.cc @@ -1792,3 +1792,199 @@ TEST(26_JoinNets) "end;\n" ); } + +// Issue #1832 - small caps are not combined properly +TEST(27_CombineSmallC) +{ + db::Netlist nl; + + db::Circuit *circuit = new db::Circuit (); + circuit->set_name ("TOP"); + nl.add_circuit (circuit); + + db::DeviceClass *device = new db::DeviceClassCapacitor (); + device->set_name ("model_name"); + nl.add_device_class (device); + + db::Net *n1 = new db::Net ("n1"); + circuit->add_net (n1); + + db::Net *n2 = new db::Net ("n2"); + circuit->add_net (n2); + + db::Net *n3 = new db::Net ("n3"); + circuit->add_net (n3); + + auto p1 = circuit->add_pin ("p1"); + auto p2 = circuit->add_pin ("p2"); + + circuit->connect_pin (p1.id (), n1); + circuit->connect_pin (p2.id (), n3); + + db::Device *c1 = new db::Device (device, "c1"); + circuit->add_device (c1); + c1->set_parameter_value (db::DeviceClassCapacitor::param_id_C, 1e-15); + + db::Device *c2 = new db::Device (device, "c2"); + circuit->add_device (c2); + c2->set_parameter_value (db::DeviceClassCapacitor::param_id_C, 2e-15); + + db::Device *c3 = new db::Device (device, "c3"); + circuit->add_device (c3); + c3->set_parameter_value (db::DeviceClassCapacitor::param_id_C, 3e-15); + + c1->connect_terminal (db::DeviceClassCapacitor::terminal_id_A, n1); + c1->connect_terminal (db::DeviceClassCapacitor::terminal_id_B, n2); + + c2->connect_terminal (db::DeviceClassCapacitor::terminal_id_A, n2); + c2->connect_terminal (db::DeviceClassCapacitor::terminal_id_B, n3); + + c3->connect_terminal (db::DeviceClassCapacitor::terminal_id_A, n1); + c3->connect_terminal (db::DeviceClassCapacitor::terminal_id_B, n3); + + EXPECT_EQ (nl.to_string (), + "circuit TOP (p1=n1,p2=n3);\n" + " device model_name c1 (A=n1,B=n2) (C=1e-15,A=0,P=0);\n" + " device model_name c2 (A=n2,B=n3) (C=2e-15,A=0,P=0);\n" + " device model_name c3 (A=n1,B=n3) (C=3e-15,A=0,P=0);\n" + "end;\n" + ); + + nl.combine_devices (); + + EXPECT_EQ (nl.to_string (), + "circuit TOP (p1=n1,p2=n3);\n" + " device model_name c1 (A=n1,B=n3) (C=3.66666666667e-15,A=0,P=0);\n" + "end;\n" + ); +} + +TEST(27_CombineSmallR) +{ + db::Netlist nl; + + db::Circuit *circuit = new db::Circuit (); + circuit->set_name ("TOP"); + nl.add_circuit (circuit); + + db::DeviceClass *device = new db::DeviceClassResistor (); + device->set_name ("model_name"); + nl.add_device_class (device); + + db::Net *n1 = new db::Net ("n1"); + circuit->add_net (n1); + + db::Net *n2 = new db::Net ("n2"); + circuit->add_net (n2); + + db::Net *n3 = new db::Net ("n3"); + circuit->add_net (n3); + + auto p1 = circuit->add_pin ("p1"); + auto p2 = circuit->add_pin ("p2"); + + circuit->connect_pin (p1.id (), n1); + circuit->connect_pin (p2.id (), n3); + + db::Device *c1 = new db::Device (device, "c1"); + circuit->add_device (c1); + c1->set_parameter_value (db::DeviceClassResistor::param_id_R, 1e-15); + + db::Device *c2 = new db::Device (device, "c2"); + circuit->add_device (c2); + c2->set_parameter_value (db::DeviceClassResistor::param_id_R, 2e-15); + + db::Device *c3 = new db::Device (device, "c3"); + circuit->add_device (c3); + c3->set_parameter_value (db::DeviceClassResistor::param_id_R, 3e-15); + + c1->connect_terminal (db::DeviceClassResistor::terminal_id_A, n1); + c1->connect_terminal (db::DeviceClassResistor::terminal_id_B, n2); + + c2->connect_terminal (db::DeviceClassResistor::terminal_id_A, n2); + c2->connect_terminal (db::DeviceClassResistor::terminal_id_B, n3); + + c3->connect_terminal (db::DeviceClassResistor::terminal_id_A, n1); + c3->connect_terminal (db::DeviceClassResistor::terminal_id_B, n3); + + EXPECT_EQ (nl.to_string (), + "circuit TOP (p1=n1,p2=n3);\n" + " device model_name c1 (A=n1,B=n2) (R=1e-15,L=0,W=0,A=0,P=0);\n" + " device model_name c2 (A=n2,B=n3) (R=2e-15,L=0,W=0,A=0,P=0);\n" + " device model_name c3 (A=n1,B=n3) (R=3e-15,L=0,W=0,A=0,P=0);\n" + "end;\n" + ); + + nl.combine_devices (); + + EXPECT_EQ (nl.to_string (), + "circuit TOP (p1=n1,p2=n3);\n" + " device model_name c1 (A=n1,B=n3) (R=1.5e-15,L=0,W=0,A=0,P=0);\n" + "end;\n" + ); +} + +TEST(27_CombineSmallL) +{ + db::Netlist nl; + + db::Circuit *circuit = new db::Circuit (); + circuit->set_name ("TOP"); + nl.add_circuit (circuit); + + db::DeviceClass *device = new db::DeviceClassInductor (); + device->set_name ("model_name"); + nl.add_device_class (device); + + db::Net *n1 = new db::Net ("n1"); + circuit->add_net (n1); + + db::Net *n2 = new db::Net ("n2"); + circuit->add_net (n2); + + db::Net *n3 = new db::Net ("n3"); + circuit->add_net (n3); + + auto p1 = circuit->add_pin ("p1"); + auto p2 = circuit->add_pin ("p2"); + + circuit->connect_pin (p1.id (), n1); + circuit->connect_pin (p2.id (), n3); + + db::Device *c1 = new db::Device (device, "c1"); + circuit->add_device (c1); + c1->set_parameter_value (db::DeviceClassInductor::param_id_L, 1e-15); + + db::Device *c2 = new db::Device (device, "c2"); + circuit->add_device (c2); + c2->set_parameter_value (db::DeviceClassInductor::param_id_L, 2e-15); + + db::Device *c3 = new db::Device (device, "c3"); + circuit->add_device (c3); + c3->set_parameter_value (db::DeviceClassInductor::param_id_L, 3e-15); + + c1->connect_terminal (db::DeviceClassInductor::terminal_id_A, n1); + c1->connect_terminal (db::DeviceClassInductor::terminal_id_B, n2); + + c2->connect_terminal (db::DeviceClassInductor::terminal_id_A, n2); + c2->connect_terminal (db::DeviceClassInductor::terminal_id_B, n3); + + c3->connect_terminal (db::DeviceClassInductor::terminal_id_A, n1); + c3->connect_terminal (db::DeviceClassInductor::terminal_id_B, n3); + + EXPECT_EQ (nl.to_string (), + "circuit TOP (p1=n1,p2=n3);\n" + " device model_name c1 (A=n1,B=n2) (L=1e-15);\n" + " device model_name c2 (A=n2,B=n3) (L=2e-15);\n" + " device model_name c3 (A=n1,B=n3) (L=3e-15);\n" + "end;\n" + ); + + nl.combine_devices (); + + EXPECT_EQ (nl.to_string (), + "circuit TOP (p1=n1,p2=n3);\n" + " device model_name c1 (A=n1,B=n3) (L=1.5e-15);\n" + "end;\n" + ); +} From ac2d20ae76f8e2956971bb785d1a4bfac7fcb6bf Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 7 Sep 2024 23:14:51 +0200 Subject: [PATCH 2/3] Implemented fix for issue #1832 (enhancements for Netlist#simplify and Netlist#combine_devices) --- src/db/db/dbCircuit.cc | 35 +++++++++++++++++++ src/db/db/dbCircuit.h | 8 +++++ src/db/db/dbNetlist.cc | 12 +++++++ src/db/db/dbNetlist.h | 8 +++++ src/db/db/gsiDeclDbNetlist.cc | 14 +++++++- src/db/unit_tests/dbNetlistTests.cc | 54 +++++++++++++++++++++++++++++ 6 files changed, 130 insertions(+), 1 deletion(-) diff --git a/src/db/db/dbCircuit.cc b/src/db/db/dbCircuit.cc index c9c4d2153e..49d3e91d5a 100644 --- a/src/db/db/dbCircuit.cc +++ b/src/db/db/dbCircuit.cc @@ -825,6 +825,41 @@ void Circuit::do_purge_nets (bool keep_pins) } } +static bool can_purge_device (const db::Device &device) +{ + if (! device.device_class ()) { + return false; + } + + const std::vector &tdefs = device.device_class ()->terminal_definitions (); + if (tdefs.size () <= 1) { + return false; + } + + const db::Net *net = device.net_for_terminal (tdefs.front ().id ()); + for (auto t = tdefs.begin () + 1; t != tdefs.end (); ++t) { + if (net != device.net_for_terminal (t->id ())) { + return false; + } + } + + return true; +} + +void Circuit::purge_devices () +{ + std::vector devices_to_be_purged; + for (device_iterator d = begin_devices (); d != end_devices (); ++d) { + if (can_purge_device (*d)) { + devices_to_be_purged.push_back (d.operator-> ()); + } + } + + for (auto d = devices_to_be_purged.begin (); d != devices_to_be_purged.end (); ++d) { + remove_device (*d); + } +} + /** * @brief Sanity check for device to be removed */ diff --git a/src/db/db/dbCircuit.h b/src/db/db/dbCircuit.h index abcb742749..9ebc5f83d2 100644 --- a/src/db/db/dbCircuit.h +++ b/src/db/db/dbCircuit.h @@ -732,6 +732,14 @@ class DB_PUBLIC Circuit */ void purge_nets_keep_pins (); + /** + * @brief Purges invalid devices + * + * This method will purge all invalid devices, i.e. those + * whose terminals are all connected to the same net. + */ + void purge_devices (); + /** * @brief Combine devices * diff --git a/src/db/db/dbNetlist.cc b/src/db/db/dbNetlist.cc index e5e8018633..a8f88d5298 100644 --- a/src/db/db/dbNetlist.cc +++ b/src/db/db/dbNetlist.cc @@ -619,6 +619,13 @@ void Netlist::purge_nets () } } +void Netlist::purge_devices () +{ + for (bottom_up_circuit_iterator c = begin_bottom_up (); c != end_bottom_up (); ++c) { + c->purge_devices (); + } +} + void Netlist::make_top_level_pins () { size_t ntop = top_circuit_count (); @@ -651,6 +658,9 @@ void Netlist::purge () Circuit *circuit = c.operator-> (); + // purge invalid devices + circuit->purge_devices (); + // purge floating, disconnected nets circuit->purge_nets (); @@ -684,6 +694,8 @@ void Netlist::simplify () { make_top_level_pins (); purge (); + + // combine devices are purge nets that are created in that step combine_devices (); purge_nets (); } diff --git a/src/db/db/dbNetlist.h b/src/db/db/dbNetlist.h index 0bff649e15..d21115838c 100644 --- a/src/db/db/dbNetlist.h +++ b/src/db/db/dbNetlist.h @@ -508,6 +508,14 @@ class DB_PUBLIC Netlist */ void purge_nets (); + /** + * @brief Purges invalid devices + * + * This method will purge all invalid devices, i.e. those + * whose terminals are all connected to the same net. + */ + void purge_devices (); + /** * @brief Creates pins for top-level circuits * diff --git a/src/db/db/gsiDeclDbNetlist.cc b/src/db/db/gsiDeclDbNetlist.cc index 1573631dcd..f8adc58392 100644 --- a/src/db/db/gsiDeclDbNetlist.cc +++ b/src/db/db/gsiDeclDbNetlist.cc @@ -1809,6 +1809,12 @@ Class decl_dbCircuit (decl_dbNetlistObject, "db", "Circuit", "For example, serial or parallel resistors can be combined into " "a single resistor.\n" ) + + gsi::method ("purge_devices", &db::Circuit::purge_devices, + "@brief Purges invalid devices.\n" + "Purges devices which are considered invalid. Such devices are for example those whose terminals are all connected to a single net.\n" + "\n" + "This method has been added in version 0.29.7." + ) + gsi::method ("purge_nets", &db::Circuit::purge_nets, "@brief Purges floating nets.\n" "Floating nets are nets with no device or subcircuit attached to. Such floating " @@ -2176,9 +2182,15 @@ Class decl_dbNetlist ("db", "Netlist", "Floating nets can be created as effect of reconnections of devices or pins. " "This method will eliminate all nets that make less than two connections." ) + + gsi::method ("purge_devices", &db::Netlist::purge_devices, + "@brief Purges invalid devices.\n" + "Purges devices which are considered invalid. Such devices are for example those whose terminals are all connected to a single net.\n" + "\n" + "This method has been added in version 0.29.7." + ) + gsi::method ("simplify", &db::Netlist::simplify, "@brief Convenience method that combines the simplification.\n" - "This method is a convenience method that runs \\make_top_level_pins, \\purge, \\combine_devices and \\purge_nets." + "This method is a convenience method that runs \\make_top_level_pins, \\purge, \\combine_devices, \\purge_devices and \\purge_nets." ) + gsi::method_ext ("read", &read_netlist, gsi::arg ("file"), gsi::arg ("reader"), "@brief Writes the netlist to the given file using the given reader object to parse the file\n" diff --git a/src/db/unit_tests/dbNetlistTests.cc b/src/db/unit_tests/dbNetlistTests.cc index 685342a3eb..47dba443c6 100644 --- a/src/db/unit_tests/dbNetlistTests.cc +++ b/src/db/unit_tests/dbNetlistTests.cc @@ -1988,3 +1988,57 @@ TEST(27_CombineSmallL) "end;\n" ); } + +TEST(28_EliminateShortedDevices) +{ + db::Netlist nl; + + db::Circuit *circuit = new db::Circuit (); + circuit->set_name ("TOP"); + nl.add_circuit (circuit); + + db::DeviceClass *device = new db::DeviceClassCapacitor (); + device->set_name ("model_name"); + nl.add_device_class (device); + + db::Net *n1 = new db::Net ("n1"); + circuit->add_net (n1); + + db::Net *n2 = new db::Net ("n2"); + circuit->add_net (n2); + + auto p1 = circuit->add_pin ("p1"); + auto p2 = circuit->add_pin ("p2"); + + circuit->connect_pin (p1.id (), n1); + circuit->connect_pin (p2.id (), n2); + + db::Device *c1 = new db::Device (device, "c1"); + circuit->add_device (c1); + c1->set_parameter_value (db::DeviceClassInductor::param_id_L, 1e-15); + + db::Device *c2 = new db::Device (device, "c2"); + circuit->add_device (c2); + c2->set_parameter_value (db::DeviceClassInductor::param_id_L, 2e-15); + + c1->connect_terminal (db::DeviceClassInductor::terminal_id_A, n1); + c1->connect_terminal (db::DeviceClassInductor::terminal_id_B, n1); + + c2->connect_terminal (db::DeviceClassInductor::terminal_id_A, n1); + c2->connect_terminal (db::DeviceClassInductor::terminal_id_B, n2); + + EXPECT_EQ (nl.to_string (), + "circuit TOP (p1=n1,p2=n2);\n" + " device model_name c1 (A=n1,B=n1) (C=1e-15,A=0,P=0);\n" + " device model_name c2 (A=n1,B=n2) (C=2e-15,A=0,P=0);\n" + "end;\n" + ); + + nl.simplify (); + + EXPECT_EQ (nl.to_string (), + "circuit TOP (p1=n1,p2=n2);\n" + " device model_name c2 (A=n1,B=n2) (C=2e-15,A=0,P=0);\n" + "end;\n" + ); +} From 074238b116593b0eecb2fc51518321042ec560c5 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sun, 8 Sep 2024 17:18:12 +0200 Subject: [PATCH 3/3] Do not include 'purge_devices' in Netlist#simplify as such devices may be intentionally included as spare or dummy devices. If you want to remove them, use 'purge_devices' on 'netlist' and/or 'schematic' --- src/db/db/dbNetlist.cc | 3 --- src/db/db/gsiDeclDbNetlist.cc | 2 +- src/db/unit_tests/dbNetlistTests.cc | 2 +- src/lvs/unit_tests/lvsTests.cc | 6 ++++++ 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/db/db/dbNetlist.cc b/src/db/db/dbNetlist.cc index a8f88d5298..3c038eb775 100644 --- a/src/db/db/dbNetlist.cc +++ b/src/db/db/dbNetlist.cc @@ -658,9 +658,6 @@ void Netlist::purge () Circuit *circuit = c.operator-> (); - // purge invalid devices - circuit->purge_devices (); - // purge floating, disconnected nets circuit->purge_nets (); diff --git a/src/db/db/gsiDeclDbNetlist.cc b/src/db/db/gsiDeclDbNetlist.cc index f8adc58392..2687fec452 100644 --- a/src/db/db/gsiDeclDbNetlist.cc +++ b/src/db/db/gsiDeclDbNetlist.cc @@ -2190,7 +2190,7 @@ Class decl_dbNetlist ("db", "Netlist", ) + gsi::method ("simplify", &db::Netlist::simplify, "@brief Convenience method that combines the simplification.\n" - "This method is a convenience method that runs \\make_top_level_pins, \\purge, \\combine_devices, \\purge_devices and \\purge_nets." + "This method is a convenience method that runs \\make_top_level_pins, \\purge, \\combine_devices and \\purge_nets." ) + gsi::method_ext ("read", &read_netlist, gsi::arg ("file"), gsi::arg ("reader"), "@brief Writes the netlist to the given file using the given reader object to parse the file\n" diff --git a/src/db/unit_tests/dbNetlistTests.cc b/src/db/unit_tests/dbNetlistTests.cc index 47dba443c6..5a992d7f05 100644 --- a/src/db/unit_tests/dbNetlistTests.cc +++ b/src/db/unit_tests/dbNetlistTests.cc @@ -2034,7 +2034,7 @@ TEST(28_EliminateShortedDevices) "end;\n" ); - nl.simplify (); + nl.purge_devices (); EXPECT_EQ (nl.to_string (), "circuit TOP (p1=n1,p2=n2);\n" diff --git a/src/lvs/unit_tests/lvsTests.cc b/src/lvs/unit_tests/lvsTests.cc index 001df63dad..d34b0fb15e 100644 --- a/src/lvs/unit_tests/lvsTests.cc +++ b/src/lvs/unit_tests/lvsTests.cc @@ -121,6 +121,12 @@ TEST(12_private) run_test (_this, "test_12.lvs", "test_12b.cir.gz", "test_12.gds.gz", true); } +TEST(121_private) +{ + // test_is_long_runner (); + run_test (_this, "test_121.lvs", "test_121.cir.gz", "test_121.gds.gz", true); +} + TEST(13_private) { // test_is_long_runner ();