From 66cdadfe4e9f0cdf4a71e74c5c576eaf4060d02f Mon Sep 17 00:00:00 2001 From: Joaquin Matres <4514346+joamatab@users.noreply.github.com> Date: Fri, 26 Jun 2026 09:27:35 +0200 Subject: [PATCH 1/4] Detect duplicate cell names before writing layouts GDS/OASIS formats require unique cell names. Add a pre-write check that raises DuplicateCellNameError with details about which names collide, helping users diagnose the issue before it corrupts output. Also suppress a ty type-checker false positive for os.sched_getaffinity (platform-dependent attribute handled at runtime via AttributeError). --- src/kfactory/conf.py | 2 +- src/kfactory/exceptions.py | 9 +++++++ src/kfactory/kcell.py | 54 +++++++++++++++++++++++++++++++++++++- 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/src/kfactory/conf.py b/src/kfactory/conf.py index 6525dc280..b274a7439 100644 --- a/src/kfactory/conf.py +++ b/src/kfactory/conf.py @@ -182,7 +182,7 @@ def get_affinity() -> int: fall back to the multiprocessing cpu count. """ try: - return len(os.sched_getaffinity(0)) + return len(os.sched_getaffinity(0)) # ty: ignore[unresolved-attribute] except AttributeError: try: import multiprocessing diff --git a/src/kfactory/exceptions.py b/src/kfactory/exceptions.py index d9fbcd04f..08b864e39 100644 --- a/src/kfactory/exceptions.py +++ b/src/kfactory/exceptions.py @@ -14,6 +14,7 @@ "CellNameError", "CrossSectionNamingConflictError", "CrossSectionSymmetryMismatchError", + "DuplicateCellNameError", "FactoriesLockedError", "InvalidLayerError", "LockedError", @@ -203,5 +204,13 @@ class CellNameError(ValueError): """Raised if a KCell is created and the automatic assigned name is taken.""" +class DuplicateCellNameError(ValueError): + """Raised when writing a layout with multiple cells sharing the same name. + + GDS/OASIS formats require unique cell names. This error provides details + about which names are duplicated and which cells are involved. + """ + + class InvalidLayerError(ValueError): """Raised when a layer is not valid.""" diff --git a/src/kfactory/kcell.py b/src/kfactory/kcell.py index 05efe4cdd..78b76e947 100644 --- a/src/kfactory/kcell.py +++ b/src/kfactory/kcell.py @@ -69,7 +69,7 @@ TAsymmetricCrossSection, TCrossSection, ) -from .exceptions import LockedError, MergeError +from .exceptions import DuplicateCellNameError, LockedError, MergeError from .geometry import DBUGeometricObject, GeometricObject, UMGeometricObject from .instance import DInstance, Instance, ProtoInstance, ProtoTInstance, VInstance from .instances import ( @@ -145,6 +145,52 @@ ] +def _check_duplicate_cell_names(layout: kdb.Layout, cell_indices: set[int]) -> None: + """Raise `DuplicateCellNameError` if any cells in `cell_indices` share a name.""" + from collections import defaultdict + + name_to_indices: dict[str, list[int]] = defaultdict(list) + for ci in cell_indices: + c = layout.cell(ci) + if c is not None and not c._destroyed(): + name_to_indices[c.name].append(ci) + + duplicates = { + name: indices for name, indices in name_to_indices.items() if len(indices) > 1 + } + if not duplicates: + return + + lines = [ + "Cannot write layout: multiple cells share the same name.", + "GDS/OASIS requires every cell name to be unique.", + "", + ] + for name, indices in sorted(duplicates.items()): + lines.append(f" {name!r} is used by {len(indices)} cells:") + for ci in indices: + c = layout.cell(ci) + if c is None: + continue + parent_count = len(list(c.each_parent_cell())) + lines.append( + f" - cell_index={ci}, {parent_count} parent(s)," + f" hierarchy_levels={c.hierarchy_levels()}" + ) + lines.append("") + + lines.append( + "This usually happens when a cell function creates or returns a cell" + " whose name collides with another cell already in the layout." + ) + lines.append( + "To catch conflicts earlier, set `kf.config.debug_names = True`" + " — this raises an error at the point where the duplicate name is assigned." + ) + + raise DuplicateCellNameError("\n".join(lines)) + + class BaseKCell(BaseModel, ABC, arbitrary_types_allowed=True): """KLayout cell and change its class to KCell. @@ -1323,6 +1369,9 @@ def write( case _: ... + relevant_cells = {self.cell_index(), *self.called_cells()} + _check_duplicate_cell_names(self.layout(), relevant_cells) + filename = str(filename) if autoformat_from_file_extension: save_options.set_format_from_filename(filename) @@ -1367,6 +1416,9 @@ def write_bytes( case _: ... + relevant_cells = {self.cell_index(), *self.called_cells()} + _check_duplicate_cell_names(self.layout(), relevant_cells) + save_options.format = save_options.format or "OASIS" save_options.clear_cells() save_options.select_cell(self.cell_index()) From 68cc348334fad898dfa028a440f26155bbb35804 Mon Sep 17 00:00:00 2001 From: Joaquin Matres <4514346+joamatab@users.noreply.github.com> Date: Fri, 26 Jun 2026 09:32:57 +0200 Subject: [PATCH 2/4] Refactor name-conflict logging and add layout-level duplicate check Deduplicate the repetitive name-conflict branches in TKCell.name setter into a single code path. Also add _check_duplicate_cell_names to KCLayout.write() so the layout-level write path catches duplicates the same way ProtoTKCell.write() does. --- src/kfactory/kcell.py | 150 ++++++++++++----------------------------- src/kfactory/layout.py | 49 +++++++++++++- 2 files changed, 90 insertions(+), 109 deletions(-) diff --git a/src/kfactory/kcell.py b/src/kfactory/kcell.py index 78b76e947..5cee388b9 100644 --- a/src/kfactory/kcell.py +++ b/src/kfactory/kcell.py @@ -528,125 +528,59 @@ def name(self, value: str) -> None: and not self.kcl.layout.cell(value).is_library_cell() and not self.is_library_cell() ): - stack = inspect.stack() - module = inspect.getmodule(stack[3].frame) tkcells = [ self.kcl.tkcells[cell.cell_index()] for cell in self.kcl.layout.cells(value) if not cell.is_library_cell() ] + conflicting = "\n".join( + f" - {tkcell.name!r} (cell_index={tkcell.kdb_cell.cell_index()}," + f" function_name={tkcell.function_name!r}," + f" basename={tkcell.basename!r})" + for tkcell in tkcells + ) + + stack = inspect.stack() + module = inspect.getmodule(stack[3].frame) + if module is not None and module.__name__ == "kfactory.layout": - frame_info = stack[5] - logger.opt(depth=2).error( - "Name conflict in " - f"{frame_info.frame.f_locals['f'].__code__.co_filename}::" - f"{frame_info.frame.f_locals['f'].__name__} at line " - f"{frame_info.frame.f_locals['f'].__code__.co_firstlineno}\n" - f"Renaming {self.name} (cell_index={self.kdb_cell.cell_index()}) to" - f" {value} would cause it to be named the same as:\n" - + "\n".join( - f" - {tkcell.name} (cell_index={tkcell.kdb_cell.cell_index()})," - f" function_name={tkcell.function_name}," - f" basename={tkcell.basename}" - for tkcell in tkcells - ) - ) - if config.debug_names: - raise ValueError( - "Name conflict in " - f"{frame_info.frame.f_locals['f'].__code__.co_filename}::" - f"{frame_info.frame.f_locals['f'].__name__} at line " - f"{frame_info.frame.f_locals['f'].__code__.co_firstlineno}\n" - f"Renaming {self.name} (cell_index={self.kdb_cell.cell_index()}" - f") to {value} would cause it to be named the same as:\n" - + "\n".join( - f" - {tkcell.name} " - f"(cell_index={tkcell.kdb_cell.cell_index()})," - f" function_name={tkcell.function_name}," - f" basename={tkcell.basename}" - for tkcell in tkcells - ) + fi = stack[5] + f_obj = fi.frame.f_locals.get("f") + if f_obj is not None: + location = ( + f"{f_obj.__code__.co_filename}::{f_obj.__name__}" + f" at line {f_obj.__code__.co_firstlineno}" ) + else: + location = f"{fi.filename}::{fi.function} at line {fi.lineno}" + log_depth = 2 else: - frame_info = stack[3] + fi = stack[3] if module is not None: - module_name = module.__name__ - if module_name == "__main__": - module_name = frame_info.filename - function_name = ( - "::" + frame_info.function - if frame_info.function != "" - else "" - ) - logger.opt(depth=3).error( - "Name conflict in " - f"{module_name}{function_name} at line " - f"{frame_info.lineno}\n" - f"Renaming {self.name} (cell_index=" - f"{self.kdb_cell.cell_index()}) to" - f" {value} would cause it to be named the same as:\n" - + "\n".join( - f" - {tkcell.name} " - f"(cell_index={tkcell.kdb_cell.cell_index()})," - f" function_name={tkcell.function_name}," - f" basename={tkcell.basename}" - for tkcell in tkcells - ) - ) - if config.debug_names: - raise ValueError( - "Name conflict in " - f"{module_name}{function_name} at line " - f"{frame_info.lineno}\n" - f"Renaming {self.name} (cell_index=" - f"{self.kdb_cell.cell_index()}) to" - f" {value} would cause it to be named the same as:\n" - + "\n".join( - f" - {tkcell.name} " - f"(cell_index={tkcell.kdb_cell.cell_index()})," - f" function_name={tkcell.function_name}," - f" basename={tkcell.basename}" - for tkcell in tkcells - ) - ) + mod_name = module.__name__ + if mod_name == "__main__": + mod_name = fi.filename else: - function_name = ( - "::" + frame_info.function - if frame_info.function != "" - else "" - ) - logger.opt(depth=3).error( - "Name conflict in " - f"{frame_info.filename}" - f"{function_name} at line {frame_info.lineno}\n" - f"Renaming {self.name} (cell_index=" - f"{self.kdb_cell.cell_index()}) to" - f" {value} would cause it to be named the same as:\n" - + "\n".join( - f" - {tkcell.name} " - f"(cell_index={tkcell.kdb_cell.cell_index()})," - f" function_name={tkcell.function_name}," - f" basename={tkcell.basename}" - for tkcell in tkcells - ) - ) - if config.debug_names: - raise ValueError( - "Name conflict in " - f"{frame_info.filename}" - f"{function_name} at line {frame_info.lineno}\n" - f"Renaming {self.name} (cell_index=" - f"{self.kdb_cell.cell_index()}) to" - f" {value} would cause it to be named the same as:\n" - + "\n".join( - f" - {tkcell.name} " - f"(cell_index={tkcell.kdb_cell.cell_index()})," - f" function_name={tkcell.function_name}," - f" basename={tkcell.basename}" - for tkcell in tkcells - ) - ) + mod_name = fi.filename + func_suffix = f"::{fi.function}" if fi.function != "" else "" + location = f"{mod_name}{func_suffix} at line {fi.lineno}" + log_depth = 3 + + msg = ( + f"Cell name conflict in {location}\n" + f"Renaming {self.name!r}" + f" (cell_index={self.kdb_cell.cell_index()}) to {value!r}" + f" would create a duplicate — the following cell(s) already" + f" have that name:\n{conflicting}\n" + f"This will make the layout unwritable (GDS/OASIS require" + f" unique cell names).\n" + f"Set `kf.config.debug_names = True` to turn this warning" + f" into an error and catch the conflict at its source." + ) + logger.opt(depth=log_depth).error(msg) + if config.debug_names: + raise DuplicateCellNameError(msg) self.kdb_cell.name = value diff --git a/src/kfactory/layout.py b/src/kfactory/layout.py index 50b445cf7..0c5da6d99 100644 --- a/src/kfactory/layout.py +++ b/src/kfactory/layout.py @@ -68,7 +68,7 @@ LayerEnclosureModel, LayerEnclosureSpec, ) -from .exceptions import FactoriesLockedError, MergeError +from .exceptions import DuplicateCellNameError, FactoriesLockedError, MergeError from .kcell import ( AnyTKCell, BaseKCell, @@ -2234,11 +2234,58 @@ def write( if kcell.is_library_cell() and not kcell.destroyed(): kcell.convert_to_static(recursive=True) + self._check_duplicate_cell_names() + if autoformat_from_file_extension: options.set_format_from_filename(filename) return self.layout.write(filename, options) + def _check_duplicate_cell_names(self) -> None: + """Raise `DuplicateCellNameError` if any cells in the layout share a name.""" + from collections import defaultdict + + name_to_cells: dict[str, list[kdb.Cell]] = defaultdict(list) + for c in self.layout.each_cell(): + if not c._destroyed(): + name_to_cells[c.name].append(c) + + duplicates = { + name: cells for name, cells in name_to_cells.items() if len(cells) > 1 + } + if not duplicates: + return + + lines = [ + "Cannot write layout: multiple cells share the same name.", + "GDS/OASIS requires every cell name to be unique.", + "", + ] + for name, cells in sorted(duplicates.items()): + lines.append(f" {name!r} is used by {len(cells)} cells:") + for c in cells: + tkcell = self.tkcells.get(c.cell_index()) + fn = tkcell.function_name if tkcell else None + bn = tkcell.basename if tkcell else None + parent_count = len(list(c.each_parent_cell())) + lines.append( + f" - cell_index={c.cell_index()}, function_name={fn!r}," + f" basename={bn!r}, {parent_count} parent(s)" + ) + lines.append("") + + lines.append( + "This usually happens when a cell function creates or returns a cell" + " whose name collides with another cell already in the layout." + ) + lines.append( + "To catch conflicts earlier, set `kf.config.debug_names = True`" + " — this raises an error at the point where the duplicate name" + " is assigned." + ) + + raise DuplicateCellNameError("\n".join(lines)) + def top_kcells(self) -> list[KCell]: """Return the top KCells.""" return [self[tc.cell_index()] for tc in self.top_cells()] From 2f34d4b910ddb0c52385b4e53a0c14e44c1f0d4a Mon Sep 17 00:00:00 2001 From: Joaquin Matres <4514346+joamatab@users.noreply.github.com> Date: Fri, 26 Jun 2026 09:54:29 +0200 Subject: [PATCH 3/4] add _deduplicate_cell_names --- src/kfactory/kcell.py | 57 ++++++++++++++++++++-------------------- src/kfactory/layout.py | 59 +++++++++++++++++++++--------------------- 2 files changed, 57 insertions(+), 59 deletions(-) diff --git a/src/kfactory/kcell.py b/src/kfactory/kcell.py index 5cee388b9..795919abd 100644 --- a/src/kfactory/kcell.py +++ b/src/kfactory/kcell.py @@ -145,8 +145,14 @@ ] -def _check_duplicate_cell_names(layout: kdb.Layout, cell_indices: set[int]) -> None: - """Raise `DuplicateCellNameError` if any cells in `cell_indices` share a name.""" +def _deduplicate_cell_names(layout: kdb.Layout, cell_indices: set[int]) -> None: + """Auto-rename cells with duplicate names so the layout can be written. + + GDS/OASIS require unique cell names. When duplicates are found among + `cell_indices`, the first cell keeps its name and subsequent ones get + a ``$1``, ``$2``, … suffix (matching KLayout's own convention). + A warning is logged for each renamed cell. + """ from collections import defaultdict name_to_indices: dict[str, list[int]] = defaultdict(list) @@ -161,34 +167,27 @@ def _check_duplicate_cell_names(layout: kdb.Layout, cell_indices: set[int]) -> N if not duplicates: return - lines = [ - "Cannot write layout: multiple cells share the same name.", - "GDS/OASIS requires every cell name to be unique.", - "", - ] - for name, indices in sorted(duplicates.items()): - lines.append(f" {name!r} is used by {len(indices)} cells:") - for ci in indices: + for name, indices in duplicates.items(): + # Keep the first cell, rename the rest + for ci in indices[1:]: c = layout.cell(ci) - if c is None: + if c is None or c._destroyed(): continue - parent_count = len(list(c.each_parent_cell())) - lines.append( - f" - cell_index={ci}, {parent_count} parent(s)," - f" hierarchy_levels={c.hierarchy_levels()}" + unique = layout.unique_cell_name(name) + was_locked = c.is_locked() + if was_locked: + c.locked = False + c.name = unique + if was_locked: + c.locked = True + logger.warning( + "Renamed duplicate cell {old!r} (cell_index={ci}) to {new!r}" + " before writing. Set `kf.config.debug_names = True` to catch" + " name conflicts earlier.", + old=name, + ci=ci, + new=unique, ) - lines.append("") - - lines.append( - "This usually happens when a cell function creates or returns a cell" - " whose name collides with another cell already in the layout." - ) - lines.append( - "To catch conflicts earlier, set `kf.config.debug_names = True`" - " — this raises an error at the point where the duplicate name is assigned." - ) - - raise DuplicateCellNameError("\n".join(lines)) class BaseKCell(BaseModel, ABC, arbitrary_types_allowed=True): @@ -1304,7 +1303,7 @@ def write( ... relevant_cells = {self.cell_index(), *self.called_cells()} - _check_duplicate_cell_names(self.layout(), relevant_cells) + _deduplicate_cell_names(self.layout(), relevant_cells) filename = str(filename) if autoformat_from_file_extension: @@ -1351,7 +1350,7 @@ def write_bytes( ... relevant_cells = {self.cell_index(), *self.called_cells()} - _check_duplicate_cell_names(self.layout(), relevant_cells) + _deduplicate_cell_names(self.layout(), relevant_cells) save_options.format = save_options.format or "OASIS" save_options.clear_cells() diff --git a/src/kfactory/layout.py b/src/kfactory/layout.py index 0c5da6d99..73b55cc8c 100644 --- a/src/kfactory/layout.py +++ b/src/kfactory/layout.py @@ -68,7 +68,7 @@ LayerEnclosureModel, LayerEnclosureSpec, ) -from .exceptions import DuplicateCellNameError, FactoriesLockedError, MergeError +from .exceptions import FactoriesLockedError, MergeError from .kcell import ( AnyTKCell, BaseKCell, @@ -2234,15 +2234,20 @@ def write( if kcell.is_library_cell() and not kcell.destroyed(): kcell.convert_to_static(recursive=True) - self._check_duplicate_cell_names() + self._deduplicate_cell_names() if autoformat_from_file_extension: options.set_format_from_filename(filename) return self.layout.write(filename, options) - def _check_duplicate_cell_names(self) -> None: - """Raise `DuplicateCellNameError` if any cells in the layout share a name.""" + def _deduplicate_cell_names(self) -> None: + """Auto-rename cells with duplicate names so the layout can be written. + + GDS/OASIS require unique cell names. The first cell keeps its name; + subsequent duplicates get ``$1``, ``$2``, … suffixes. A warning is + logged for each rename. + """ from collections import defaultdict name_to_cells: dict[str, list[kdb.Cell]] = defaultdict(list) @@ -2256,35 +2261,29 @@ def _check_duplicate_cell_names(self) -> None: if not duplicates: return - lines = [ - "Cannot write layout: multiple cells share the same name.", - "GDS/OASIS requires every cell name to be unique.", - "", - ] - for name, cells in sorted(duplicates.items()): - lines.append(f" {name!r} is used by {len(cells)} cells:") - for c in cells: + for name, cells in duplicates.items(): + for c in cells[1:]: + if c._destroyed(): + continue + unique = self.layout.unique_cell_name(name) tkcell = self.tkcells.get(c.cell_index()) fn = tkcell.function_name if tkcell else None - bn = tkcell.basename if tkcell else None - parent_count = len(list(c.each_parent_cell())) - lines.append( - f" - cell_index={c.cell_index()}, function_name={fn!r}," - f" basename={bn!r}, {parent_count} parent(s)" + was_locked = c.is_locked() + if was_locked: + c.locked = False + c.name = unique + if was_locked: + c.locked = True + logger.warning( + "Renamed duplicate cell {old!r} (cell_index={ci}," + " function_name={fn!r}) to {new!r} before writing." + " Set `kf.config.debug_names = True` to catch name" + " conflicts earlier.", + old=name, + ci=c.cell_index(), + fn=fn, + new=unique, ) - lines.append("") - - lines.append( - "This usually happens when a cell function creates or returns a cell" - " whose name collides with another cell already in the layout." - ) - lines.append( - "To catch conflicts earlier, set `kf.config.debug_names = True`" - " — this raises an error at the point where the duplicate name" - " is assigned." - ) - - raise DuplicateCellNameError("\n".join(lines)) def top_kcells(self) -> list[KCell]: """Return the top KCells.""" From 81def666bf0d8eabfc22a715169bfbcc01414c71 Mon Sep 17 00:00:00 2001 From: Joaquin Matres <4514346+joamatab@users.noreply.github.com> Date: Fri, 26 Jun 2026 09:56:25 +0200 Subject: [PATCH 4/4] Use hasattr guard for os.sched_getaffinity Replaces try/except AttributeError with a hasattr check so that ty can narrow the type on both macOS (attribute absent) and Linux (attribute present) without needing a platform-specific ignore comment. --- src/kfactory/conf.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/kfactory/conf.py b/src/kfactory/conf.py index b274a7439..bf4e40bd5 100644 --- a/src/kfactory/conf.py +++ b/src/kfactory/conf.py @@ -181,15 +181,14 @@ def get_affinity() -> int: On (most) linux we can get it through the scheduling affinity. Otherwise, fall back to the multiprocessing cpu count. """ + if hasattr(os, "sched_getaffinity"): + return len(os.sched_getaffinity(0)) try: - return len(os.sched_getaffinity(0)) # ty: ignore[unresolved-attribute] - except AttributeError: - try: - import multiprocessing + import multiprocessing - return multiprocessing.cpu_count() - except ModuleNotFoundError: - return 1 + return multiprocessing.cpu_count() + except ModuleNotFoundError: + return 1 dotenv_path = find_dotenv(usecwd=True)