Repository: KLayout/klayout (language: python)

Issue:
Allow the usage of `Cell.shapes(LayerInfo(...))`
Hi Matthias,

I'll cut to the chase directly: Would it be possible to allow `Cell.shapes` to take a LayerInfo object and have it resolve it to the layer_index internally? If easily possible this would also be nice for `Cell.begin_shapes_rec(...)` and its friends. But those are less affected than shapes due to infrequent use of those.

_The longer story:_

I am currently working on expanding the use of (gds)pdks (well, pdk is a very advanced word for where we are in photonics, but you get the general idea)  across multiple layouts (in order to essentially  separate cells into more or less standard cells and cells generated specifically for the tapeout such as new structures for tests etc).

One of the big struggles I encountered is that most of my end-users do not really understand why  a layer index is kind of bound to the layout. So, I have essentially rewritten (or rather hidden deeper in the code)  an enum for the layer indexes to mainly use LayerInfo objects instead.

But now I have the problem of having to explain to users why they have to do `c.shapes(c.layout().find_layer(LayerInfo(...)))` (of course this is a bit less explicit than here, but for illustration purposes it's in pure klayout code).

Best,
Sebastian


Discussion / hints:
Hi @sebastian-goeldi,

That is possible of course, but there are many places where layer indexes are used and some don't have a layout.

If you need support for explaining this decision - here is the motivation: There are anonymous layers. Whenever you do some computations, you may encounter cases when you need temporary layers. DRC decks use plenty of them for example. A temporary layer does not have coordinates like layer or datatype number. So LayerInfo is not a good way of identifying them. The layer index is more generic and allows addressing anonymous layers too.

Matthias
Hi Matthias,

Yes, I am fully aware that there are cases.

I am merely requesting this as I think it would be generally a good usability improvement for pure layout work (as in constructing the layout), and less so for any checking/verification work.

It would be nice to have the layoutinfo options as second class citizens alongside the index, purely on the cell functions (and maybe layout functions) where applicable.

But if you think it's not really feasible or out of scope, that's totally fine. To me (from a very surface perspective view), it seems like it wouldn't be a gigantic change, but I could totally be wrong. Feel free to close the issue in that case.
I mean in a few places it is actually easy to implement. Some BTW for cell_index vs. Cell object. It just won't go everywhere, because often, the index is the only information available and there is no Layout object in sight.

Maybe I have a different style of script coding, that it never occurred to me that the `LayoutInfo` option would be useful. Almost all scripts I write have some structure like this:

```
ly = ... # Layout

metal1 = ly.layer(30, 0)
via1   = ly.layer(31, 0)
metal2 = ly.layer(32, 0)
...


# later I write something like this:
cell.shapes(metal1).insert(polygon)
```

I like having symbolic names for the layers rather than GDS layer numbers.

So the `LayerInfo` option would allow me code that is only marginally different:

```
ly = ... # Layout

metal1 = pya.LayerInfo(30, 0)
via1   = pya.LayerInfo((31, 0)
metal2 = pya.LayerInfo((32, 0)
...


# later I write something like this:
cell.shapes(metal1).insert(polygon)
```

The only advantage I see is that you can take these constants from some generic module that does not need access to a Layout object. But I solved that with a little trick:

```
layers = {
  "metal1": pya.LayerInfo(30, 0),
  "via1":   pya.LayerInfo(31, 0),
  "metal2": pya.LayerInfo(32, 0),
}

ly = ... # Layout

for k in layers.keys():
  locals()[k] = ly.layer(layers[k])

...

# later I write something like this:
cell.shapes(metal1).insert(polygon)
```

Matthias



Yes, this is something that I essentially did before. I can illustrate in a simplified way why I would love LayerInfos:

In kfactory (and by extension gdsfactory) we have a cell decorator which collects args and kwargs automatically and converts them to MetaInfo objects on write of the gds (and stores them in `kfactory:settings:{(kw)arg_name}`. This is the weaker reason why I would love LayerInfo objects.

The bigger reason is the following:

We started using something we now call photonic design kits. You can think of these as you would of a standard cell library in digital. But since we cannot rely on device extraction for LVS, we have to do cell recognition (and build a schematic from there). We usually have a mix of circuits made out of these standard(ized) cells and test structures. In order to facilitate and make recognition easy, I made it so that the design kit has its own layout object and is actually a full library.

Then in the tapeout we instantiate these library cells

```python
cell.create_inst(library_cell_function(width=1000, length=10_000, layer=layer1)
```

The problem is, if you use layer indexes for this, if you use a layer_index from `cell.layout()` and pass it to this function, you will be in trouble as best case you might get the same layer, or a different layer, or worst case nothing because the index doesn't exist in the library.

Therefore, I have rewritten kfactory to allow defining the layers by LayerInfo objects and not (extended) `int` enums anymore.


Also, with regards to the tricks. These usually work very nicely if it's contained by scripts, but since we have many people that contribute to our tapeouts, and many of these people are not experienced programmers at all, I need something that is very compatible with IDE autocomplete and ideally also passes static type-checkers like mypy without essentially having to disable them on almost all lines (which will be necessary if you do dictionary comprehension and similar as the type checker will not be able to follow this).
I think I got the point ...

Just a detail: in case the LayerInfo desribes a layer that does not exist yet: should the function create it or raise an error? For const versions the layer can' t be created, but for non-const it can.

I feel the layer should be created (if possible) as in GDS there is no difference between an empty and a missing layer.

I also feel that is a kind of substantial change for a minor release, but it's backward compatible and hell, why not.

Matthias
I would be ok with either of those choices! I lean towards raising an error (to kinda keep it similar to an invalid index), but creating it is also totally ok.

Whether major or minor release, I will also leave it up to you. If you follow the semantic versioning logic, I think it would still qualify for minor release, as it's only adding new API. But again, I think this should be up to you.

Fix patch:
diff --git a/src/db/db/gsiDeclDbCell.cc b/src/db/db/gsiDeclDbCell.cc
index c3c053550..ef5f5b2db 100644
--- a/src/db/db/gsiDeclDbCell.cc
+++ b/src/db/db/gsiDeclDbCell.cc
@@ -1802,6 +1802,39 @@ read_simple (db::Cell *cell, const std::string &path)
   return read_options (cell, path, db::LoadLayoutOptions ());
 }
 
+static db::Shapes &shapes_with_layer_info (db::Cell *cell, const db::LayerProperties &info)
+{
+  if (! cell->layout ()) {
+    throw tl::Exception (tl::to_string (tr ("Cell is not associated with a layout")));
+  }
+
+  unsigned int li = cell->layout ()->get_layer (info);
+  return cell->shapes (li);
+}
+
+static const db::Shapes &shapes_with_layer_info_const (const db::Cell *cell, const db::LayerProperties &info)
+{
+  if (! cell->layout ()) {
+    throw tl::Exception (tl::to_string (tr ("Cell is not associated with a layout")));
+  }
+
+  int li = cell->layout ()->get_layer_maybe (info);
+  if (li < 0) {
+    throw tl::Exception (tl::sprintf (tl::to_string (tr ("%s is not a valid layer within the layout of the cell")), info.to_string ()));
+  }
+
+  return cell->shapes ((unsigned int) li);
+}
+
+static void clear_layer_with_info (db::Cell *cell, const db::LayerProperties &info)
+{
+  if (cell->layout ()) {
+    int layer = cell->layout ()->get_layer_maybe (info);
+    if (layer >= 0) {
+      cell->clear ((unsigned int) layer);
+    }
+  }
+}
 
 static db::Point default_origin;
 
@@ -1992,22 +2025,46 @@ Class<db::Cell> decl_Cell ("db", "Cell",
     "This method gives access to the shapes list on a certain layer.\n"
     "If the layer does not exist yet, it is created.\n"
     "\n"
-    "@param index The layer index of the shapes list to retrieve\n"
+    "@param layer_index The layer index of the shapes list to retrieve\n"
     "\n"
     "@return A reference to the shapes list\n"
   ) +
+  gsi::method_ext ("shapes", shapes_with_layer_info, gsi::arg ("layer"),
+    "@brief Returns the shapes list of the given layer\n"
+    "\n"
+    "This version takes a \\LayerInfo object and will look up the layer index. If no layer exists "
+    "with these attributes, it will be created.\n"
+    "\n"
+    "@param layer The layer attributes\n"
+    "\n"
+    "@return A reference to the shapes list\n"
+    "\n"
+    "This variant has been introduced in version 0.29.7.\n"
+  ) +
   gsi::method_ext ("shapes", &shapes_of_cell_const, gsi::arg ("layer_index"),
     "@brief Returns the shapes list of the given layer (const version)\n"
     "\n"
     "This method gives access to the shapes list on a certain layer. This is the const version - only const (reading) methods "
     "can be called on the returned object.\n"
     "\n"
-    "@param index The layer index of the shapes list to retrieve\n"
+    "@param layer_index The layer index of the shapes list to retrieve\n"
     "\n"
     "@return A reference to the shapes list\n"
     "\n"
     "This variant has been introduced in version 0.26.4.\n"
   ) +
+  gsi::method_ext ("shapes", shapes_with_layer_info_const, gsi::arg ("layer"),
+    "@brief Returns the shapes list of the given layer (const version)\n"
+    "\n"
+    "This version takes a \\LayerInfo object and will look up the layer index. An error is raised if "
+    "no layer with these attributes exists.\n"
+    "\n"
+    "@param layer The layer attributes\n"
+    "\n"
+    "@return A reference to the shapes list\n"
+    "\n"
+    "This variant has been introduced in version 0.29.7.\n"
+  ) +
   gsi::method ("clear_shapes", &db::Cell::clear_shapes,
     "@brief Clears all shapes in the cell\n"
   ) +
@@ -2082,6 +2139,13 @@ Class<db::Cell> decl_Cell ("db", "Cell",
   gsi::method ("clear", &db::Cell::clear, gsi::arg ("layer_index"),
     "@brief Clears the shapes on the given layer\n"
   ) +
+  gsi::method_ext ("clear", &clear_layer_with_info, gsi::arg ("layer"),
+    "@brief Clears the shapes on the given layer\n"
+    "\n"
+    "This version takes a \\LayerInfo object for the layer. If no such layer exists, this method does nothing.\n"
+    "\n"
+    "This variant has been introduced in version 0.29.7.\n"
+  ) +
   gsi::method_ext ("clear", &clear_all,
     "@brief Clears the cell (deletes shapes and instances)\n"
     "This method has been introduced in version 0.23.\n"
diff --git a/src/db/db/gsiDeclDbLayout.cc b/src/db/db/gsiDeclDbLayout.cc
index 3edf80298..e7b58f3aa 100644
--- a/src/db/db/gsiDeclDbLayout.cc
+++ b/src/db/db/gsiDeclDbLayout.cc
@@ -1103,6 +1103,30 @@ void break_polygons1 (db::Layout *layout, size_t max_vertex_count, double max_ar
   db::break_polygons (*layout, max_vertex_count, max_area_ratio);
 }
 
+void delete_layer_from_info (db::Layout *layout, const db::LayerProperties &info)
+{
+  int li = layout->get_layer_maybe (info);
+  if (li >= 0) {
+    layout->delete_layer ((unsigned int) li);
+  }
+}
+
+void clear_layer_from_info (db::Layout *layout, const db::LayerProperties &info)
+{
+  int li = layout->get_layer_maybe (info);
+  if (li >= 0) {
+    layout->clear_layer ((unsigned int) li);
+  }
+}
+
+void clear_layer_from_info_with_flags (db::Layout *layout, const db::LayerProperties &info, unsigned int flags)
+{
+  int li = layout->get_layer_maybe (info);
+  if (li >= 0) {
+    layout->clear_layer ((unsigned int) li, flags);
+  }
+}
+
 Class<db::Layout> decl_Layout ("db", "Layout",
   gsi::constructor ("new", &layout_ctor_with_manager, gsi::arg ("manager"),
     "@brief Creates a layout object attached to a manager\n"
@@ -1963,8 +1987,18 @@ Class<db::Layout> decl_Layout ("db", "Layout",
     "\n"
     "This method was introduced in version 0.19.\n"
     "\n"
-    "@param layer_index The index of the layer to delete.\n"
+    "@param layer_index The index of the layer to clear.\n"
   ) +                               
+  gsi::method_ext ("clear_layer", &clear_layer_from_info, gsi::arg ("layer"),
+    "@brief Clears a layer\n"
+    "\n"
+    "This variant takes a \\LayerInfo object to address the layer.\n"
+    "It does nothing if no layer with these attributes exists.\n"
+    "\n"
+    "This variant was introduced in version 0.26.7.\n"
+    "\n"
+    "@param layer The attributes of the layer to clear.\n"
+  ) +
   gsi::method ("clear_layer", static_cast<void (db::Layout::*) (unsigned int, unsigned int)> (&db::Layout::clear_layer), gsi::arg ("layer_index"), gsi::arg ("flags"),
     "@brief Clears a layer (given shape types only)\n"
     "\n"
@@ -1972,9 +2006,19 @@ Class<db::Layout> decl_Layout ("db", "Layout",
     "\n"
     "This method was introduced in version 0.28.9.\n"
     "\n"
-    "@param layer_index The index of the layer to delete.\n"
+    "@param layer_index The index of the layer to clear.\n"
     "@param flags The type selector for the shapes to delete (see \\Shapes class, S... constants).\n"
   ) +
+  gsi::method_ext ("clear_layer", &clear_layer_from_info_with_flags, gsi::arg ("layer"), gsi::arg ("flags"),
+    "@brief Clears a layer (given shape types only)\n"
+    "\n"
+    "This variant takes a \\LayerInfo object to address the layer.\n"
+    "It does nothing if no layer with these attributes exists.\n"
+    "\n"
+    "This variant was introduced in version 0.26.7.\n"
+    "\n"
+    "@param layer The attributes of the layer to clear.\n"
+  ) +
   gsi::method ("delete_layer", &db::Layout::delete_layer, gsi::arg ("layer_index"),
     "@brief Deletes a layer\n"
     "\n"
@@ -1983,6 +2027,16 @@ Class<db::Layout> decl_Layout ("db", "Layout",
     "\n"
     "@param layer_index The index of the layer to delete.\n"
   ) +
+  gsi::method_ext ("delete_layer", &delete_layer_from_info, gsi::arg ("layer"),
+    "@brief Deletes a layer\n"
+    "\n"
+    "This variant takes a \\LayerInfo object to address the layer.\n"
+    "It does nothing if no layer with these attributes exists.\n"
+    "\n"
+    "This variant was introduced in version 0.26.7.\n"
+    "\n"
+    "@param layer The attributes of the layer to delete.\n"
+  ) +
   gsi::method_ext ("layer_indexes|#layer_indices", &layer_indexes,
     "@brief Gets a list of valid layer's indices\n"
     "This method returns an array with layer indices representing valid layers.\n"


Test patch:
diff --git a/testdata/ruby/dbCellTests.rb b/testdata/ruby/dbCellTests.rb
index 2111ed388..b0fc91d0f 100644
--- a/testdata/ruby/dbCellTests.rb
+++ b/testdata/ruby/dbCellTests.rb
@@ -98,6 +98,32 @@ def test_2
 
   end
 
+  # methods with LayerInfo instead layer index
+  def test_3
+
+    ly = RBA::Layout::new
+    top = ly.create_cell("TOP")
+
+    l1 = ly.layer(1, 0)
+    top.shapes(RBA::LayerInfo::new(1, 0)).insert(RBA::Box::new(0, 0, 100, 200))
+
+    assert_equal(top.shapes(l1).size, 1)
+
+    # unknown layers are ignored in clear
+    top.clear(RBA::LayerInfo::new(2, 0))
+    assert_equal(top.shapes(l1).size, 1)
+
+    # clear with LayerInfo
+    top.clear(RBA::LayerInfo::new(1, 0))
+    assert_equal(top.shapes(l1).size, 0)
+
+    # layer is created if not there
+    assert_equal(ly.layer_infos, [ RBA::LayerInfo::new(1, 0) ])
+    top.shapes(RBA::LayerInfo::new(2, 0)).insert(RBA::Box::new(0, 0, 100, 200))
+    assert_equal(ly.layer_infos, [ RBA::LayerInfo::new(1, 0), RBA::LayerInfo::new(2, 0) ])
+
+  end
+
 end
 
 load("test_epilogue.rb")
diff --git a/testdata/ruby/dbLayoutTests1.rb b/testdata/ruby/dbLayoutTests1.rb
index 6acbd2474..b0693188d 100644
--- a/testdata/ruby/dbLayoutTests1.rb
+++ b/testdata/ruby/dbLayoutTests1.rb
@@ -2144,6 +2144,43 @@ def shapes2str(shapes)
 
   end
 
+  # Methods taking LayoutInfo instead of layer index
+  def test_26
+
+    ly = RBA::Layout::new
+    top = ly.create_cell("TOP")
+    l1 = ly.layer(1, 0)
+
+    # ignored
+    ly.clear_layer(RBA::LayerInfo::new(2, 0))
+
+    top.shapes(l1).insert(RBA::Box::new(0, 0, 100, 200))
+    assert_equal(top.shapes(l1).size, 1)
+
+    ly.clear_layer(RBA::LayerInfo::new(1, 0))
+    assert_equal(top.shapes(l1).size, 0)
+
+    top.shapes(l1).insert(RBA::Box::new(0, 0, 100, 200))
+    assert_equal(top.shapes(l1).size, 1)
+
+    ly.clear_layer(RBA::LayerInfo::new(1, 0), RBA::Shapes::SPolygons)
+    assert_equal(top.shapes(l1).size, 1)
+
+    ly.clear_layer(RBA::LayerInfo::new(1, 0), RBA::Shapes::SBoxes)
+    assert_equal(top.shapes(l1).size, 0)
+
+    assert_equal(ly.layer_infos, [ RBA::LayerInfo::new(1, 0) ])
+
+    # ignored
+    ly.delete_layer(RBA::LayerInfo::new(2, 0))
+    assert_equal(ly.layer_infos, [ RBA::LayerInfo::new(1, 0) ])
+    
+    ly.delete_layer(RBA::LayerInfo::new(1, 0))
+    assert_equal(ly.layer_infos, [ ])
+
+  end
+
+
   # Iterating while flatten
   def test_issue200