gdsfactory / kfactory

gdsfactory with a klayout backend
https://gdsfactory.github.io/kfactory/
MIT License
31 stars 12 forks source link

Support `kcl.cell(include_module=True)` #437

Closed 95-martin-orion closed 1 month ago

95-martin-orion commented 2 months ago

Is your feature request related to a problem? Please describe. gdsfactory v7.* supported include_module in cells, which allows the same function name to be used for cells in different modules without worrying about collisions. This is distinct from kcl.cell(basename="foo"), which cannot automatically include the name of the function in the cell name.

Describe the solution you'd like Add the include_module behavior from gf.cell in gdsfactory v7.* to kcl.cell and provide access to it in gdsfactory.

For example, this should work:

# In module `foo`, with gdsfactory>=8.0
import gdsfactory as gf

@gf.cell(include_module=True)
def my_comp() -> gf.Component:
    ...

my_comp()  # Creates a cell named e.g. `foo_my_comp`
sebastian-goeldi commented 2 months ago

I think this is just fighting symptoms and actually a bad idea:

Imagine the following:

bend.py ```python import kfactory as kf @kf.kcl.cell(include_module=True) def euler(radius: kf.kf_types.um, width: kf.kf_types.um, layer: kf.LayerEnum, enclosure: kf.LayerEnclosue) -> kf.KCell: c = kf.kcl.kcell() # ... return c @kf.kcl.cell def mycell() -> kf.KCell: c = kf.kcl.kcell() c << euler(args1) # ... return c ```
other.py ```python @kf.kcl.cell def myothercell() -> kf.KCell: c = kf.kcl.kcell() c << kf.cells.euler.bend_euler(args1) # ... return c ```
main.py ```python import kfactory as kf from bend import mycell from other import myothercell @kf.kcl.cell def mymaincell() -> kf.KCell: c = kf.kcl.kcell() c << mycell() c << myothercell() # ... return c mymaincell().show() ```

I would not want to debug this in a project

95-martin-orion commented 2 months ago

Our project follows the Google style guide for Python, which prohibits importing functions directly. This is useful because it means you don't have to check your entire repo for mycell() to see if you can define it in other.py - the module can always be used to differentiate it.

If the issue is instead that {module}_{function} names can collide with existing cells (e.g. bend_euler above), this can be avoided with alternate naming schemes. For example, {module}-{function} cannot collide with an existing function name because - is not a valid character in Python function names.

sebastian-goeldi commented 2 months ago

https://www.klayout.de/forum/discussion/comment/9963/#Comment_9963

- is not necessarily a safe character to use in gds.

95-martin-orion commented 2 months ago

Thanks for the context. From that thread, it looks like $ might be a safe alternative, although {module}${function} isn't as clear to read. Double underscore (__) could also work, since these should be rare in function names.

This feature is important to our project because we often have variants of a component for different designs, and a layout may import multiple designs. Including the version in the component name (e.g. Foo_v00_euler) is tedious, but manageable; the real issue comes in when someone forgets to do this and the cache silently replaces the component with a different version. We've had similar unexpected-caching issues happen before, and they're very difficult to track down.

sebastian-goeldi commented 2 months ago

You can actually very easily patch this in yourself, which I would recommend in this case as I am a bit hesitant to just implement this, as it will make debugging a nightmare:

test2.py ```python from typing import Annotated, ParamSpec import kfactory as kf CellParams = ParamSpec("CellParams") def module_cell( _func: kf.kcell.KCellFunc[kf.kcell.KCellParams], ) -> kf.kcell.KCellFunc[kf.kcell.KCellParams]: mod = _func.__module__ if mod != "__main__": basename = mod + "_" + _func.__name__ else: basename = _func.__name__ return kf.cell(basename=basename)(_func) @module_cell def straight(width: Annotated[float, "um"], length: Annotated[float, "um"]) -> kf.KCell: # Note: Annotated is not necessary, this just allows to store units in the metainfos c = kf.kcl.kcell() c.shapes(c.kcl.layer(1, 0)).insert(kf.kdb.DBox(width, length)) return c ```
main.py ```python import kfactory as kf from test import module_cell from test import straight as test1_straight from typing import Annotated @module_cell def straight(width: Annotated[float, "um"], length: Annotated[float, "um"]) -> kf.KCell: c = kf.kcl.kcell() c.shapes(c.kcl.layer(1, 0)).insert(kf.kdb.DBox(width, length)) return c c = kf.KCell() c << straight(2, 10) c << test1_straight(2, 10) c.show() ```

When running main.py, this will yield: image

95-martin-orion commented 2 months ago

Thanks for the detailed recommendation! That should be enough to unblock my team's upgrade to v8.

sebastian-goeldi commented 2 months ago

@joamatab what do you think about this?

I am hesitant to implement this as a flag in the cell decorator, but I would be willing to implement this as a separate decorator (e.g. as in the example above)

joamatab commented 2 months ago

i think it would be nice to implement this

sebastian-goeldi commented 1 month ago

Initial testing is at #451 but it's not ready yet.