feat: add inline schematic functions for all 23 device cells#117
feat: add inline schematic functions for all 23 device cells#117ThomasPluck merged 14 commits intomainfrom
Conversation
Extract simulation metadata from c.info["vlsir"] dicts into proper DSchematic functions linked via @gf.cell(schematic_function=...). Each schematic function returns a DSchematic with tags, symbol, ports (directional mapping), and models (SPICE info with sections and params). Cells updated across 7 files: - fet_transistors.py: nmos, pmos, nmos_hv, pmos_hv - rf_transistors.py: rfnmos, rfpmos, rfnmos_hv, rfpmos_hv - bjt_transistors.py: npn13G2, npn13G2L, npn13G2V, pnpMPA - resistors.py: rsil, rppd, rhigh - capacitors.py: cmim, rfcmim - passives.py: svaricap, esd_nmos, ptap1, ntap1 - antennas.py: dantenna, dpantenna Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reviewer's GuideIntroduce DSchematic-based inline schematic functions for all device and fixed cells, wire them through the gf.cell(schematic_function=...) decorator, and remove legacy vlsir-based simulation metadata and tests in favor of schematic-layer SPICE models. Sequence diagram for creating a cell with gf.cell and DSchematic-based schematicsequenceDiagram
actor User
participant GFCellDecorator as gf_cell_decorator
participant LayoutCell as nmos
participant SchematicFn as nmos_schematic
participant DS as DSchematic
participant LayoutComp as gf_Component
participant Simulator as spice_simulator
User->>GFCellDecorator: call nmos(width, length, nf, m)
GFCellDecorator->>LayoutCell: invoke layout body
LayoutCell-->>GFCellDecorator: return LayoutComp (GDS layout)
Note over GFCellDecorator,SchematicFn: When schematic is needed (netlisting/schematic view)
GFCellDecorator->>SchematicFn: call nmos_schematic(width, length, nf, m)
SchematicFn->>DS: DSchematic()
SchematicFn->>DS: set info.tags, info.symbol
SchematicFn->>DS: set info.ports (directional mapping)
SchematicFn->>DS: set info.models (SPICE metadata)
SchematicFn->>DS: create_port(D, G, S, B)
DS-->>SchematicFn: DSchematic instance
SchematicFn-->>GFCellDecorator: DSchematic instance
GFCellDecorator-->>User: LayoutComp (with associated schematic_function)
User->>Simulator: request simulation for nmos instance
Simulator->>DS: read models, port_order, params
Simulator-->>User: SPICE results based on schematic-layer metadata
Class diagram for gf.cell-decorated layout cells and DSchematic-based schematicsclassDiagram
class GFCellDecorator {
+schematic_function: function
+__call__(cell_function)
}
class DSchematic {
+info: dict
+create_port(name, cross_section, x, y, orientation)
}
class LayoutCell_nmos {
+width: float
+length: float
+nf: int
+m: int
+__call__(width, length, nf, m) gf.Component
}
class Schematic_nmos {
+width: float
+length: float
+nf: int
+m: int
+__call__(width, length, nf, m) DSchematic
}
class LayoutCell_rfnmos {
+width: float
+length: float
+nf: int
+m: int
+__call__(width, length, nf, m, cnt_rows, met2_cont, gat_ring, guard_ring) gf.Component
}
class Schematic_rfnmos {
+width: float
+length: float
+nf: int
+m: int
+cnt_rows: int
+met2_cont: bool
+gat_ring: bool
+guard_ring: str
+__call__(width, length, nf, m, cnt_rows, met2_cont, gat_ring, guard_ring) DSchematic
}
class LayoutCell_cmim {
+width: float
+length: float
+__call__(width, length) gf.Component
}
class Schematic_cmim {
+width: float
+length: float
+__call__(width, length) DSchematic
}
class SchematicModels {
+language: str
+name: str
+spice_type: str
+library: str
+sections: list
+port_order: list
+params: dict
}
class LegacyVlsirMetadata {
-model: str
-spice_type: str
-spice_lib: str
-port_order: list
-port_map: dict
-params: dict
}
GFCellDecorator --> LayoutCell_nmos : decorates
GFCellDecorator --> LayoutCell_rfnmos : decorates
GFCellDecorator --> LayoutCell_cmim : decorates
GFCellDecorator --> Schematic_nmos : schematic_function
GFCellDecorator --> Schematic_rfnmos : schematic_function
GFCellDecorator --> Schematic_cmim : schematic_function
Schematic_nmos --> DSchematic : returns
Schematic_rfnmos --> DSchematic : returns
Schematic_cmim --> DSchematic : returns
DSchematic "1" o-- "*" SchematicModels : models
LayoutCell_nmos .. LegacyVlsirMetadata : c.info.vlsir removed
LayoutCell_rfnmos .. LegacyVlsirMetadata : c.info.vlsir removed
LayoutCell_cmim .. LegacyVlsirMetadata : c.info.vlsir removed
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Several schematic functions accept a
modelargument (e.g.,nmos_schematic,cmim_schematic,esd_nmos_schematic) but ignore it and hardcode the SPICEnamefield; either use the parameter to select the model or drop it to avoid confusion. - The
_XS = "metal1_routing"constant is duplicated across multiple modules; consider defining this once in a shared place (or pulling fromTECHif appropriate) to avoid divergence between files. - There is a lot of nearly identical boilerplate in the new
*_schematicfunctions (tags/symbol/ports/model structure/port creation); consider introducing small helpers or factories for MOS/BJT/passive schematics to reduce repetition and the chance of inconsistent metadata.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Several schematic functions accept a `model` argument (e.g., `nmos_schematic`, `cmim_schematic`, `esd_nmos_schematic`) but ignore it and hardcode the SPICE `name` field; either use the parameter to select the model or drop it to avoid confusion.
- The `_XS = "metal1_routing"` constant is duplicated across multiple modules; consider defining this once in a shared place (or pulling from `TECH` if appropriate) to avoid divergence between files.
- There is a lot of nearly identical boilerplate in the new `*_schematic` functions (tags/symbol/ports/model structure/port creation); consider introducing small helpers or factories for MOS/BJT/passive schematics to reduce repetition and the chance of inconsistent metadata.
## Individual Comments
### Comment 1
<location path="ihp/cells/fet_transistors.py" line_range="511-520" />
<code_context>
# Public cell functions
# ---------------------------------------------------------------------------
-@gf.cell
+def nmos_schematic(
+ width: float = 0.15,
+ length: float = 0.13,
+ nf: int = 1,
+ m: int = 1,
+ model: str = "sg13_lv_nmos",
+) -> DSchematic:
+ s = DSchematic()
+ s.info["tags"] = ["transistor", "mos", "lv"]
+ s.info["symbol"] = "nmos"
+ s.info["ports"] = {"left": ["G"], "right": ["D", "S"], "bottom": ["B"]}
+ s.info["models"] = [
+ {
+ "language": "spice",
+ "name": "sg13_lv_nmos",
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The `model` parameter is not reflected in the schematic model definition, which can cause mismatches when users override it.
In `nmos_schematic` (and the other new `*_schematic` helpers), the `model` argument is accepted but the SPICE model is still hard-coded to `"sg13_lv_nmos"` in `s.info["models"]`. Any non-default `model` passed by callers is therefore ignored.
To make the API consistent, either:
- use `model` for the `name` field (with the current value as the default), or
- remove `model` from the signature if overriding it is not supported.
The same fix should be applied consistently to the other schematic helpers (PMOS, HV devices, RF MOS, varicap, ESD, resistor, capacitor, etc.).
Suggested implementation:
```python
s.info["models"] = [
{
"language": "spice",
"name": model,
```
Apply the same pattern to all other `*_schematic` helper functions in this file (and related files if present):
1. Ensure each helper that accepts a `model: str = "..."` parameter uses that parameter for the SPICE model name, i.e. replace any hard-coded `"name": "some_model_name"` with `"name": model`.
2. Confirm the default value of `model` in the function signature matches the existing hard-coded model name to preserve current behavior for callers that do not override it.
3. If any schematic helper does not intend to support model overriding, remove the unused `model` parameter from its signature instead of wiring it through.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@sebastian-goeldi to fix schematic_function kwarg in cell decorator issue |
| "library": "sg13g2_moslv_mod.lib", | ||
| "sections": ["tt", "ff", "ss", "sf", "fs"], |
There was a problem hiding this comment.
I don't think these are the right library and corners for many of these models
There was a problem hiding this comment.
In my PR I started a guide for LLMs to do this accurately
https://github.com/gdsfactory/IHP/pull/116/changes#diff-7ea36bdf0b626bbbb77c515d561cc9c23dca190060d7de12920ebebf5ac9719e
In particular for IHP you need the corner files as the top level to set up the properties correctly and you need to use whatever section is actually in that file eg mos_tt
There was a problem hiding this comment.
Probably not, point is to lay the foundation, get everyone on the same page about PDK modifications.
There was a problem hiding this comment.
@pepijndevos please review that you're happy with corner data and then merge.
|
As per gdsfactory/gdsfactory#4455 - this seems to be the way to go, going to close out #116 and work to get this inline with GF 9.39.3 and tests/pre-commit passing |
- Update gdsfactory dependency to >=9.39.3 (required for schematic_function kwarg) - Add inline schematic functions for all 23 device cells following metadata_guide.md conventions (IHP tags, MOSFET port orientation, corner file libraries, string params) - Migrate ~32 fixed.py deprecated cells and bondpads.py from c.info["vlsir"] to @gf.cell(schematic_function=...) pattern - Remove primitives.py (schematic-only elements handled elsewhere) - Remove models/to_vlsir.py and vlsir/vlsirtools dependencies - Fix rfcmim wrapper to reset c.info["model"] after copying from inner cell - Regenerate test regression YAMLs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Latest push: VLSIR removal + schematic metadata cleanupBuilding on the initial schematic function work, the latest commit ( Removed
Updated
Verification
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Several schematic factories have mismatches between
info['ports'], the ports created on the DSchematic, and themodels[...]['port_order'](e.g.svaricap_schematicdefines aBNport but omits it fromports, andrsil_schematic/rppd_schematic/rhigh_schematicincludebninport_orderbut never create a corresponding port); please align the declared ports andport_orderfor all devices so layout/schematic/export stay consistent. - The D/G/S/B (and some other) ports use orientations that don't match their geometric positions in several schematics (e.g. D at y=1 with orientation 180, S at y=-1 with orientation 0); it would be clearer and less error‑prone to standardize orientations (e.g. top=90, bottom=270, left=180, right=0) across all DSchematic definitions.
- In
bondpad_schematic(and a few others) model parameters are encoded as Python expressions in strings (e.g."shape": '{"octagon": 0, "square": 1, "circle": 2}[shape]'); consider normalizing these to concrete values inside the schematic function so downstream consumers of the SPICE metadata don't need to evaluate Python code embedded in strings.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Several schematic factories have mismatches between `info['ports']`, the ports created on the DSchematic, and the `models[...]['port_order']` (e.g. `svaricap_schematic` defines a `BN` port but omits it from `ports`, and `rsil_schematic`/`rppd_schematic`/`rhigh_schematic` include `bn` in `port_order` but never create a corresponding port); please align the declared ports and `port_order` for all devices so layout/schematic/export stay consistent.
- The D/G/S/B (and some other) ports use orientations that don't match their geometric positions in several schematics (e.g. D at y=1 with orientation 180, S at y=-1 with orientation 0); it would be clearer and less error‑prone to standardize orientations (e.g. top=90, bottom=270, left=180, right=0) across all DSchematic definitions.
- In `bondpad_schematic` (and a few others) model parameters are encoded as Python expressions in strings (e.g. `"shape": '{"octagon": 0, "square": 1, "circle": 2}[shape]'`); consider normalizing these to concrete values inside the schematic function so downstream consumers of the SPICE metadata don't need to evaluate Python code embedded in strings.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- Standardize port orientations: top=90, bottom=270, left=180, right=0 (fixes 8 FET/RF-FET schematics that had swapped orientations) - Add missing BN port to BJT npn schematics, svaricap, rfcmim, and all three resistor schematics — aligns info["ports"], create_port calls, and port_order - Align resistor port_order to use DSchematic port names (P1/P2/BN) instead of SPICE pin names (1/2/bn) - Normalize bondpad shape param from embedded Python dict expression to concrete integer value Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addressed Sourcery feedbackAll three items from the second Sourcery review are resolved in 1. Port mismatches — aligned
|
Copied from IHP-Open-PDK so the PDK package is self-contained for ngspice simulation. These can be compiled to OSDI with openvaf. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use root relative paths and add verilog models
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fixed cells without pcell equivalents had docstrings referencing non-existent `ihp.cells.X()` instead of `ihp.cells.X_fixed()`. Also excluded palace_demo_cpw notebook from execution as it depends on an external service that returns unexpected status values. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mosaic doesn't recognize "subcircuit" as a device type. Use "ckt" which renders as a generic box with ports. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
docs still fail |
|
We need to change the ports to the new format i can do it tomorrow morning and maybe look at what's up with the docs |
|
Got off a call with JKU today, it seems to a pretty common complaint that the pcells are directional. There is a solution to this in kf.Pin() - idk if there's time or bandwidth for this, however. |
|
Wdym directional? |
|
Ports have an orientation in GDSFactory, this is enforced by gdsfactory's own routers which causes headaches. |
Migrate from dict-of-lists format ({"top": ["D"], ...}) to the Mosaic
PortEntry schema ([{"name": "D", "side": "top", "type": "electric"}, ...])
across all 9 cell definition files (43 port specs total).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The fnmatch pattern "palace_demo_cpw" doesn't match "palace_demo_cpw.ipynb", causing the notebook to execute and fail on the missing TOPMETAL2 layer. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
DSchematicfunctions inline in each cell file, linked via@gf.cell(schematic_function=...)decoratortags,symbol,ports(directional mapping), andmodels(SPICE metadata with sections, port_order, and computed params)c.info["vlsir"]blocks from layout cell bodies — metadata now lives exclusively in the schematic layerCells updated (23 across 7 files)
nmos,pmos,nmos_hv,pmos_hvrfnmos,rfpmos,rfnmos_hv,rfpmos_hvnpn13G2,npn13G2L,npn13G2V,pnpMPArsil,rppd,rhighcmim,rfcmimsvaricap,esd_nmos,ptap1,ntap1dantenna,dpantennaTest plan
@gf.cell(schematic_function=...)decorator is forwarded once upstream fix landspytest tests/ -xto confirm no regressions🤖 Generated with Claude Code
Summary by Sourcery
Introduce inline schematic definitions for IHP device cells and migrate simulation metadata from layout components to the schematic layer.
New Features:
Enhancements:
Build:
Documentation:
Tests: