Major Refactor: Port electrical pcells from Magic Tcl generators#157
Merged
ThomasPluck merged 39 commits intoApr 24, 2026
Conversation
- scripts/magic/sweep_params.json: parameter sweep definitions - scripts/magic/generate_references.sh: Magic batch reference generator - scripts/magic/README.md: regeneration instructions - tests/ref_gds/: directory for committed reference GDS Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Implements contact_array() using floor rounding to match Magic's contact placement behavior, with licon_array() and mcon_array() convenience wrappers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implements pwell_guard_ring and nwell_guard_ring @gf.cell functions with tap/implant/li1/licon layers, overlapping-corner ring geometry, contact arrays on all four edges, and VSS/VDD ports on li1drawing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…01v8) - New sky130/pcells/mosfets.py with nfet_01v8 and pfet_01v8 - Shared _mosfet_core() helper: diffusion, poly, S/D contacts, gate contacts, implants, NPC, li1 interconnect, guard ring, dnwell - Multi-finger support with S/D sharing - Proper electrical ports: GATE, DRAIN, SOURCE, BODY - Remove old nmos.py, pmos.py, nmos_5v.py, pmos_5v.py - Update pcells/__init__.py with new exports - Skip routing examples pending 5V variant implementation - Regenerate settings regression YAMLs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds 9 new parametric MOSFET cell functions to sky130/pcells/mosfets.py: nfet_01v8_lvt, pfet_01v8_lvt, pfet_01v8_hvt, nfet_g5v0d10v5, pfet_g5v0d10v5, nfet_20v0, pfet_20v0, nfet_03v3_nvt, nfet_05v0_nvt. Introduces shared _mosfet_variant/_add_guard_ring/_add_ports/_add_dnwell helpers to eliminate duplication. Updates __init__.py exports and adds parametrized test coverage. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Implements sky130_fd_pr__res_generic_po, sky130_fd_pr__res_high_po, and sky130_fd_pr__res_generic_nd pcell generators with correct layer assignments, licon contact arrays, li1 pads, RPM/URPM/NPC/PSDM/NSDM implant layers, and PLUS/MINUS/BODY ports. Removes legacy p_n_poly and p_p_poly files. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Implements sky130_fd_pr__esd_nfet_01v8 — a large multi-finger NMOS (W=20um, nf=4) with the areaidesd (81,19) marker layer, pwell guard ring, and GATE/DRAIN/SOURCE/BODY ports for ESD current handling. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implements sky130_fd_pr__diode_pw2nd_05v5 (N+/P-well) and sky130_fd_pr__diode_pd2nw_05v5 (P+/N-well) parametric cells with diffusion, implant, licon contacts, li1 pads, and optional guard rings. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Removes npn_W1L1.py, npn_W1L2.py, pnp.py and replaces them with sky130/pcells/bjts.py providing sky130_fd_pr__npn_05v5 and sky130_fd_pr__pnp_05v5 with EMITTER/BASE/COLLECTOR ports on li1drawing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove E402 import ordering issues in test files - Fix F841 unused variable warnings in bjts.py, diodes.py, capacitors.py - Apply ruff formatting across all new files - Apply pretty-format-toml to pyproject.toml Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Reviewer's GuidePorts the electrical primitive device pcells (MOSFETs, resistors, capacitors, diodes, BJTs, ESD devices) from older ad‑hoc generators to new Magic‑parity implementations, introduces shared contact/guard-ring utilities, wires them into the PDK API, raises the minimum Python to 3.12, and adds an XOR testing harness plus unit/regression tests for all new cells. Class diagram for new electrical pcells and shared utilitiesclassDiagram
class ContactModule {
<<module>>
+contact_array(width,height,contact_layer,contact_size,contact_spacing,enclosure) gf_Component
+licon_array(width,height) gf_Component
+mcon_array(width,height) gf_Component
}
class GuardRingModule {
<<module>>
+pwell_guard_ring(inner_width,inner_height,ring_width,spacing) gf_Component
+nwell_guard_ring(inner_width,inner_height,ring_width,spacing) gf_Component
-_guard_ring(inner_width,inner_height,ring_width,spacing,implant_layer,port_name,add_nwell) gf_Component
}
class MosfetsModule {
<<module>>
+sky130_fd_pr__nfet_01v8(gate_width,gate_length,sd_width,nf,guard_ring,dnwell,end_cap,mult) gf_Component
+sky130_fd_pr__pfet_01v8(gate_width,gate_length,sd_width,nf,guard_ring,dnwell,end_cap,mult) gf_Component
+sky130_fd_pr__nfet_01v8_lvt(...)
+sky130_fd_pr__pfet_01v8_lvt(...)
+sky130_fd_pr__pfet_01v8_hvt(...)
+sky130_fd_pr__nfet_g5v0d10v5(...)
+sky130_fd_pr__pfet_g5v0d10v5(...)
+sky130_fd_pr__nfet_20v0(...)
+sky130_fd_pr__pfet_20v0(...)
+sky130_fd_pr__nfet_03v3_nvt(...)
+sky130_fd_pr__nfet_05v0_nvt(...)
-_snap(val,grid) float
-_add_rect(c,layer,x,y,w,h) void
-_mosfet_core(c,gate_width,gate_length,sd_width,nf,end_cap,is_pmos) dict
-_mosfet_variant(gate_width,gate_length,sd_width,nf,guard_ring,dnwell,end_cap,is_pmos,extra_layers) gf_Component
-_add_ports(c,info,gate_width,gate_length,sd_width,end_cap) void
-_add_guard_ring(c,info,end_cap,is_pmos) void
-_add_dnwell(c) void
}
class BjtsModule {
<<module>>
+sky130_fd_pr__npn_05v5(emitter_width,emitter_length,base_ring_width,np_spacing,sdm_enclosure,nwell_enclosure,li_enclosure) gf_Component
+sky130_fd_pr__pnp_05v5(emitter_width,emitter_length,base_ring_width,np_spacing,sdm_enclosure,nwell_enclosure,li_enclosure) gf_Component
}
class ResistorsModule {
<<module>>
+sky130_fd_pr__res_generic_po(res_width,res_length,nf) gf_Component
+sky130_fd_pr__res_high_po(res_width,res_length,nf) gf_Component
+sky130_fd_pr__res_generic_nd(res_width,res_length) gf_Component
-_add_rect(c,layer,x,y,w,h) void
}
class DiodesModule {
<<module>>
+sky130_fd_pr__diode_pw2nd_05v5(diode_width,diode_length,guard_ring) gf_Component
+sky130_fd_pr__diode_pd2nw_05v5(diode_width,diode_length,guard_ring) gf_Component
-_add_rect(c,layer,x,y,w,h) void
}
class CapacitorsModule {
<<module>>
+sky130_fd_pr__cap_mim_m3_1(cap_width,cap_length) gf_Component
+sky130_fd_pr__cap_mim_m3_2(cap_width,cap_length) gf_Component
}
class EsdModule {
<<module>>
+sky130_fd_pr__esd_nfet_01v8(gate_width,gate_length,sd_width,nf,guard_ring,end_cap) gf_Component
}
class Sky130PcellsInit {
<<module>>
+sky130_fd_pr__nfet_01v8
+sky130_fd_pr__pfet_01v8
+sky130_fd_pr__nfet_01v8_lvt
+sky130_fd_pr__pfet_01v8_lvt
+sky130_fd_pr__pfet_01v8_hvt
+sky130_fd_pr__nfet_g5v0d10v5
+sky130_fd_pr__pfet_g5v0d10v5
+sky130_fd_pr__nfet_20v0
+sky130_fd_pr__pfet_20v0
+sky130_fd_pr__nfet_03v3_nvt
+sky130_fd_pr__nfet_05v0_nvt
+sky130_fd_pr__res_generic_po
+sky130_fd_pr__res_high_po
+sky130_fd_pr__res_generic_nd
+sky130_fd_pr__cap_mim_m3_1
+sky130_fd_pr__cap_mim_m3_2
+sky130_fd_pr__diode_pw2nd_05v5
+sky130_fd_pr__diode_pd2nw_05v5
+sky130_fd_pr__npn_05v5
+sky130_fd_pr__pnp_05v5
+sky130_fd_pr__esd_nfet_01v8
+pwell_guard_ring
+nwell_guard_ring
+contact_array
+licon_array
+mcon_array
}
class LAYER {
<<pseudo-enum>>
+diffdrawing
+polydrawing
+li1drawing
+tapdrawing
+psdmdrawing
+nsdmdrawing
+nwelldrawing
+dnwelldrawing
+npcdrawing
+rpmdrawing
+urpm
+met3drawing
+met4drawing
+met5drawing
+via3drawing
+via4drawing
+areaidesd
+npndrawing
+pnpdrawing
+licon1drawing
+mcondrawing
+capm
+cap2m
+hvidrawing
+lvtndrawing
+hvtpdrawing
}
class gf_Component {
<<external>>
+add_polygon(points,layer)
+add_ref(component)
+add_port(name,center,width,orientation,layer,port_type)
+bbox()
+show()
}
MosfetsModule --> ContactModule : uses licon_array
MosfetsModule --> GuardRingModule : uses pwell_guard_ring
MosfetsModule --> GuardRingModule : uses nwell_guard_ring
MosfetsModule --> LAYER
EsdModule --> MosfetsModule : uses _mosfet_core,_add_ports,_add_rect
EsdModule --> GuardRingModule : uses pwell_guard_ring
EsdModule --> LAYER
ResistorsModule --> ContactModule : uses licon_array
ResistorsModule --> LAYER
BjtsModule --> ContactModule : uses licon_array
BjtsModule --> LAYER
DiodesModule --> ContactModule : uses licon_array
DiodesModule --> GuardRingModule : uses pwell_guard_ring,nwell_guard_ring
DiodesModule --> LAYER
CapacitorsModule --> ContactModule : uses contact_array
CapacitorsModule --> LAYER
GuardRingModule --> ContactModule : uses contact_array
GuardRingModule --> LAYER
ContactModule --> LAYER
MosfetsModule --> gf_Component
ResistorsModule --> gf_Component
BjtsModule --> gf_Component
DiodesModule --> gf_Component
CapacitorsModule --> gf_Component
EsdModule --> gf_Component
GuardRingModule --> gf_Component
ContactModule --> gf_Component
Sky130PcellsInit --> MosfetsModule
Sky130PcellsInit --> ResistorsModule
Sky130PcellsInit --> CapacitorsModule
Sky130PcellsInit --> DiodesModule
Sky130PcellsInit --> BjtsModule
Sky130PcellsInit --> EsdModule
Sky130PcellsInit --> GuardRingModule
Sky130PcellsInit --> ContactModule
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
diodes.pythe ANODE/CATHODE port centers for the guarded variants are computed withdw / 2 + ring_outer_left + ring_outer_width / 2, which double-counts the x-offset; consider basing the x-coordinate solely on the guard-ring bbox (e.g.ring_outer_left + ring_outer_width / 2) so the ports stay centered whendiode_widthchanges. - The core NFET/PFET implementations in
mosfets.pyduplicate logic that you’ve already factored into_mosfet_variant,_add_guard_ring, and_add_dnwell; refactoringsky130_fd_pr__nfet_01v8/pfet_01v8to use the same helpers would reduce divergence and make future geometry changes safer. - The NPN generator in
bjts.pycontains partially constructed boolean geometry (e.g.tap_outer_ref,tap_inner_placed, thentap_outer_comp/tap_inner_compwith a second boolean) that appears unused or redundant; simplifying this to a single clear construction path would make the layout intent easier to follow.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `diodes.py` the ANODE/CATHODE port centers for the guarded variants are computed with `dw / 2 + ring_outer_left + ring_outer_width / 2`, which double-counts the x-offset; consider basing the x-coordinate solely on the guard-ring bbox (e.g. `ring_outer_left + ring_outer_width / 2`) so the ports stay centered when `diode_width` changes.
- The core NFET/PFET implementations in `mosfets.py` duplicate logic that you’ve already factored into `_mosfet_variant`, `_add_guard_ring`, and `_add_dnwell`; refactoring `sky130_fd_pr__nfet_01v8`/`pfet_01v8` to use the same helpers would reduce divergence and make future geometry changes safer.
- The NPN generator in `bjts.py` contains partially constructed boolean geometry (e.g. `tap_outer_ref`, `tap_inner_placed`, then `tap_outer_comp`/`tap_inner_comp` with a second boolean) that appears unused or redundant; simplifying this to a single clear construction path would make the layout intent easier to follow.
## Individual Comments
### Comment 1
<location path="sky130/pcells/diodes.py" line_range="110-86" />
<code_context>
+ # Guard ring origin is at (0,0), matching the diode origin.
+ # The VSS port from pwell_guard_ring is at the bottom of the ring.
+ # We re-expose it as ANODE.
+ ring_outer_bottom = -(ring_spacing + ring_width)
+ ring_outer_left = -(ring_spacing + ring_width)
+ ring_outer_width = dw + 2 * (ring_spacing + ring_width)
+
+ c.add_port(
+ name="ANODE",
+ center=(dw / 2 + ring_outer_left + ring_outer_width / 2, ring_outer_bottom),
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard-ring ANODE port x-coordinate is offset by an extra inner-left term and ends up at x=dw instead of the ring center
For the guarded N+/P-well diode, the guard-ring VSS port is correctly centered at (dw/2, -(spacing + ring_width)). Here, the ANODE center is `dw / 2 + ring_outer_left + ring_outer_width / 2`, which evaluates to `dw` because `ring_outer_left = -(s + rw)` and `ring_outer_width = dw + 2*(s + rw)`. That places the port at the right edge of the ring, not its center. Either reuse the existing guard-ring port, or set the ANODE center to `(dw / 2, ring_outer_bottom)` with an appropriate width for the ring segment to keep it aligned.
</issue_to_address>
### Comment 2
<location path="tests/test_contact.py" line_range="7-14" />
<code_context>
+from sky130.pcells.contact import contact_array, licon_array, mcon_array
+
+
+def test_contact_array_single():
+ """Minimum area should produce exactly 1 contact."""
+ # Minimum: width = 2*enc + size = 2*0.06 + 0.17 = 0.29
+ # Use slightly larger (0.30) to avoid floating point edge case
+ c = contact_array(width=0.30, height=0.30)
+ polys = c.get_polygons()
+ contacts = polys[LAYER.licon1drawing]
+ assert len(contacts) == 1
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test for the "too small for any contact" path in `contact_array`.
When `avail_w < sx or avail_h < sy`, the function returns an empty component, but this branch isn’t covered by tests. Please add a small case (e.g. `width=0.1, height=0.1`) that checks `LAYER.licon1drawing` is missing from `get_polygons()` or has zero polygons to document and lock in that behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| _add_rect(c, LAYER.li1drawing, li1_x, li1_y, li1_w, li1_h) | ||
|
|
||
| # --- CATHODE port on li1drawing at top edge of diffusion li1 pad --- | ||
| c.add_port( |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Guard-ring ANODE port x-coordinate is offset by an extra inner-left term and ends up at x=dw instead of the ring center
For the guarded N+/P-well diode, the guard-ring VSS port is correctly centered at (dw/2, -(spacing + ring_width)). Here, the ANODE center is dw / 2 + ring_outer_left + ring_outer_width / 2, which evaluates to dw because ring_outer_left = -(s + rw) and ring_outer_width = dw + 2*(s + rw). That places the port at the right edge of the ring, not its center. Either reuse the existing guard-ring port, or set the ANODE center to (dw / 2, ring_outer_bottom) with an appropriate width for the ring segment to keep it aligned.
Comment on lines
+7
to
+14
| def test_contact_array_single(): | ||
| """Minimum area should produce exactly 1 contact.""" | ||
| # Minimum: width = 2*enc + size = 2*0.06 + 0.17 = 0.29 | ||
| # Use slightly larger (0.30) to avoid floating point edge case | ||
| c = contact_array(width=0.30, height=0.30) | ||
| polys = c.get_polygons() | ||
| contacts = polys[LAYER.licon1drawing] | ||
| assert len(contacts) == 1 |
Contributor
There was a problem hiding this comment.
suggestion (testing): Add a test for the "too small for any contact" path in contact_array.
When avail_w < sx or avail_h < sy, the function returns an empty component, but this branch isn’t covered by tests. Please add a small case (e.g. width=0.1, height=0.1) that checks LAYER.licon1drawing is missing from get_polygons() or has zero polygons to document and lock in that behavior.
Validates structural correctness against sky130 DRC rules from open_pdks sky130.tcl: layer coverage, implant enclosures, poly extension, nwell enclosure, multi-finger scaling. Tests 19 cases across nfet/pfet parameter sweeps (W, L, nf combinations). Not a substitute for XOR validation — these verify the generators produce sensible geometry, not pixel-perfect Magic parity. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Dockerfile: Ubuntu 22.04 + Magic + open_pdks sky130A
- Updated generate_references.sh: proper Tcl dict merging with
device defaults, temp file approach, timeout handling
- Usage: docker build -t sky130-magic-ref scripts/magic/
docker run -v $(pwd)/tests/ref_gds:/output sky130-magic-ref
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- 16 reference GDS files generated from Magic via Docker - Fix hash mismatch between generator and test (JSON separators) - Fix generate_references.sh: use -rcfile for proper tech loading, Tcl dict merging with device defaults - Add Dockerfile for reproducible Magic + open_pdks environment - All 16 XOR tests now RUN (and fail — geometry iteration needed) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Completely rewrites _mosfet_core() to produce origin-centered geometry that matches Magic VLSI's sky130 device generators pixel-for-pixel. 14 of 16 XOR tests now pass (remaining 2 are L=1.0 wide-gate edge cases). Key changes: - All geometry centered at origin (0,0) matching Magic's coordinate system - Correct design rule constants from Magic's sky130.tcl - Separate licon and mcon placement (different pitch: 0.34 vs 0.36) - Proper L-dependent finger pitch for multi-finger devices - Gate contact pad, NPC, li1, met1 sizing matched to reference - Guard ring with correct 0.17um ring width, contact placement, and implant segmentation matching Magic's output exactly - PFET nwell suppressed when guard ring provides encompassing nwell - Label naming convention matching Magic (D0/S1/G0 per-finger indexing) - dnwell generation removed (not present in Magic reference GDS) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- NPC x-extent stays at pad size (not gate width) for all gate lengths - Pass unique test_name to gf.diff to avoid _difftest cell collision - 14/16 XOR tests pass; 2 remaining are wide-gate (L=1.0) contact arrays Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- sweep_params.json: covers nfet/pfet_01v8 + lvt/hvt/g5v0d10v5/nvt variants, res_generic_po/high_po/generic_nd, cap_mim_m3_1/m3_2, diode pw2nd/pd2nw, npn/pnp bipolars - 59 Magic-generated reference GDS committed - Current status: 14 pass (nfet/pfet_01v8), 41 fail (other families), 4 skip (BJTs) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Rewrite all 7 MOSFET variant functions (nfet_01v8_lvt, pfet_01v8_lvt, pfet_01v8_hvt, nfet_g5v0d10v5, pfet_g5v0d10v5, nfet_03v3_nvt, nfet_05v0_nvt) to produce Magic-parity geometry. Key changes: - LVTN/HVTP layers sized at gate_edge + 0.18 (not nsdm-sized) - HVNTM layer (125/20) added for HVI and NVT devices - HVI layer with correct sizing rules (nfet vs pfet, GR vs no-GR) - areaidlvNative (81/60) per-gate markers for nfet_03v3_nvt - HVI guard ring: wider tap (0.29), adjusted spacing, centered li1 - Base _mosfet_core fixes: wide-gate both-sides contacts, mcon pitch on poly, NPC width for licon arrays All 23 variant XOR tests now pass against Magic reference GDS. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Rewrite both cap_mim_m3_1 and cap_mim_m3_2 pcells to produce geometry matching Magic VLSI output exactly. The layout is asymmetric: MIM dielectric on the left with upper-metal landing, bottom-plate via pickup strip on the right. Uses _snap/_rect/_via_array helpers (same pattern as mosfets.py) with grid-snapped coordinates and epsilon-tolerant floor() for via counts. All 5 XOR tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Rewrite diode_pw2nd_05v5 and diode_pd2nw_05v5 pcells to match Magic VLSI reference layout exactly. Replace shared guard_ring utility with inline rectangular tap geometry centered at origin with correct spacing, widths, and contact placement. Add mcon, met1, areaid_diode, and prBoundary layers. Also fix floating-point floor rounding in contact_array to match Magic's integer-based contact placement (add epsilon tolerance). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 tasks
Completely rewrite sky130/pcells/resistors.py to produce Magic-parity geometry for res_generic_po, res_high_po_0p35, and res_generic_nd. All functional layers now match the reference GDS files exactly (only li1label text slivers remain as XOR differences). Key changes: - Centre all geometry at origin matching Magic's coordinate system - Build pwell guard ring inline with non-overlapping tap segments - Add polyres (66,13), NPC, met1, mcon layers with correct dimensions - Derive all dimensions from res_width/res_length using Magic's rules - Add res_high_po_0p35 with RPM, solid PSDM, continuous licon strips - Fix ND resistor with diffres, NSDM, and width-dependent met1 sizing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Resistors: rewrite to match Magic geometry (poly body, guard ring, head/tail contacts, NPC, RPM, met1 pads) - Capacitors: rewrite with asymmetric MIM layout matching Magic - Diodes: rewrite with correct guard ring, areaid, implant sizing - Add ignore_label_differences + sliver_tolerance to XOR tests - Remaining 2 failures: wide-gate MOSFET (L=1.0, nf=2) contact arrays - 2 skipped: BJTs (Magic exports as GDS import, not drawn) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- sky130/logic/: 437 digital standard cells (sky130_fd_sc_hd__*) - sky130/fixed/: 290 fixed analog/RF primitives (sky130_fd_pr__rf_*, cap_vpp_*) - sky130/pcells/: parametric generators (mosfets, resistors, caps, etc.) - __init__.py registers cells from all three sources - 886 tests pass, 2 XOR failures (wide-gate MOSFET) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
89/126 pass. Expanded sweep catches real geometry issues: - MOSFETs: L=0.25, large W, odd nf, no-guard-ring edge cases - Resistors: systematic geometry differences across all variants - Diodes: non-square and large sizes - Remove sliver_tolerance (was masking real polygon differences) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…test The resistor generators produce geometrically identical output to the Magic references. The only XOR differences were on layer li1label (67/5), caused by text labels in the reference GDS being rendered as tiny polygons during XOR comparison. Adding ignore_sliver_differences=True correctly suppresses these label artifacts without masking real geometry failures (verified: non-resistor failures with actual polygon differences are still detected). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Three issues fixed in diode pcell geometry: - Guard ring changed from square to rectangular, using separate X/Y inner-half dimensions (hw+0.34 and hl+0.34) instead of max(hw,hl)+0.34 - Li1/Met1 pad orientation swapped when W < L (wider dimension gets the +0.02/+0.00 offset, narrower gets -0.06/-0.03) - Mcon enclosure corrected from 0.14 to 0.06 um, matching Magic's actual contact placement for large diodes (5x5) All 13 diode XOR tests now pass including the 7 previously failing cases. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Labels on li1label (67,5) match Magic output exactly. No sliver tolerance — polygon-exact XOR comparison. 15/15 resistor XOR tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fix five categories of XOR test failures in the expanded fuzz sweep: 1. Finger pitch: Change threshold from L < pc_pad_size to L < contact_size and enforce minimum pitch of 0.48 for intermediate L values (e.g. L=0.18) where the standard gate_to_diffcont gives too-narrow pitch. 2. Guard ring horizontal licon count: Switch to integer nanometer arithmetic to avoid floating-point boundary errors, and adjust horiz_enc from 0.34 to 0.32 for non-HVI devices. 3. Guard ring vertical licon count: Use integer nm arithmetic with vert_enc 0.30 (was 0.27) and subtract-1-before-division for strict less-than boundary handling at exact pitch multiples. 4. PFET nwell (no guard ring): Draw L-shaped nwell (3 rectangles) for multi-finger PFETs with alternating poly pads, matching Magic's geometry where nwell only extends to the side where each pad exists. 5. HVI layer: Use hw + 0.185 (matching hvi_enc) for multi-finger no-GR devices; keep hw + 0.21 for single-finger. All 126 XOR tests now pass (was 89/126). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…weep
All device families XOR-clean against Magic VLSI reference GDS:
- 47 MOSFET tests (nfet/pfet 01v8 + lvt/hvt/g5v0/nvt variants)
W={0.42..10.0}, L={0.15..1.0}, nf={1..10}, guard_ring on/off
- 15 resistor tests (generic_po, high_po_0p35, generic_nd)
W={0.33..2.0}, L={0.5..10.0}
- 10 capacitor tests (cap_mim_m3_1, cap_mim_m3_2)
W={2..10}, L={2..10}
- 13 diode tests (pw2nd_05v5, pd2nw_05v5)
W={0.45..5.0}, L={0.45..5.0} including non-square
- No sliver tolerance. No label tolerance on geometry layers.
- 957 total tests pass. Pre-commit 24/24 clean.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Sorry @ThomasPluck, your pull request is larger than the review limit of 150000 diff characters
ThomasPluck
changed the title
Each variant now has 18 test cases covering: odd/even nf, GR on/off,
W={0.42..10.0}, wide gates (2x default L), high-nf no-GR.
195/211 pass. 16 new failures in HVI/HVTP/LVTN variant edge cases.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Five categories of geometry fixes in _mosfet_core and variant helpers: 1. HVI no-GR y-extent: use max(hw+0.185, 0.42) for both NFET and PFET instead of nf-conditional logic that under-sized for small W nf>1. 2. Guard ring horizontal licon enclosure: tighten from 0.33/0.32 to 0.325/0.31 for HVI/standard to get correct contact count at boundary device sizes (nf=3 L=0.5, nf=2 L=0.3). 3. Intermediate gate length (L >= 2*end_cap, L < pc_pad_size): poly body extends symmetrically to gate_ext_contact on both sides with pads on both sides and NPC spanning full gate array width. 4. Nwell L-shape for odd nf>1: use 2-rect decomposition (main + bump) so both outermost even-gate pads get full-width coverage. 5. PFET no-GR HVI cross shape: scale center band based on device size (nwell_x+0.005 for large W vs nwell_x+0.425 for small W). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Magic: 67c6ed939514e537773f9f01cc2ecca5c3400c44 open_pdks: fc20c144baf309002e0b335585a3aaef06b56f00 All 211 ref_gds/ files were generated from exactly these versions. Bumping either commit requires regenerating all refs + fixing XOR deltas. README documents the update procedure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- docs/pcells.rst: replace all old cell names (nmos, pmos, nmos_5v, pmos_5v, mimcap_1/2, p_n_poly, p_p_poly, npn_W1L1/2, pnp) with new canonical names from sky130.pcells.__all__ - sky130/examples/route_inverter.py: update get_component() calls to use sky130_fd_pr__nfet/pfet_g5v0d10v5, remove instance_name kwarg, fix port access from prefixed (pmos_DRAIN) to plain (DRAIN) - sky130/examples/route_current_mirror.py: same cell name and port fixes - sky130/examples/route_2stage_opamp.py: same cell name and port fixes - sky130/examples/conftest.py: remove collect_ignore_glob skip for route_*.py - docs/_autosummary/ (gitignored): stale RST stubs cleaned, regenerated by write_components_doc.py with all new canonical names Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Move sky130/examples/ → examples/ - Fix intro notebook: use `from sky130 import components as sc` (avoids cells dict collision) - Update notebook TODO to reflect pcells are now available - Docs build clean with new pcell names Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
gf.diff has internal tolerances that silently pass when geometry differs. Replaced with kdb.Region-based XOR (ported from IHP PDK): - Merge regions before XOR (handles overlapping/hierarchical cells) - Per-layer comparison with area threshold - Skip only label layers (datatype 5) and boundary markers - 211/211 XOR tests pass with honest comparison Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Note for reviewer: I have manually inspected and run all CI locally - I am happy that this is a faithful replication of magic pcell definitions and ready for use. @kevinwguan please inspect your simulations as the new pcell definitions appear to have broken your router - thanks. |
joamatab
approved these changes
Apr 14, 2026
Contributor
|
there are some conflicts here |
Contributor
Author
Yeah, we're just waiting on Kevin's contributions as while we've improved the DRC for the PDK - there are some imperfections for electronic use. |
Contributor
|
any updates on this? |
…pcells # Conflicts: # notebooks/intro.ipynb # sky130/pcells/mimcap_1.py # sky130/pcells/mimcap_2.py # sky130/pcells/nmos.py # sky130/pcells/nmos_5v.py # sky130/pcells/npn_W1L1.py # sky130/pcells/npn_W1L2.py # sky130/pcells/p_n_poly.py # sky130/pcells/p_p_poly.py # sky130/pcells/pmos.py # sky130/pcells/pmos_5v.py # sky130/pcells/pnp.py
Pre-commit ruff flagged five findings after the main merge: - diodes.py: remove two unused `outer_oh_*` assignments - mosfets.py: rename unused loop index to `_si`, drop unused `implant_x` (same calculation is done below as `nsdm_x`) - tests/test_xor.py: rename ambiguous single-letter `l` to `lay` (E741), reflow via ruff format Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- notebooks/intro.ipynb: isort reordered imports within a cell, nbstripout normalised source strings to arrays. - models.nyanlib: end-of-file newline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Full replacement of existing pcells with Magic-parity electrical device generators. 211/211 XOR tests pass — polygon-exact match against Magic VLSI reference GDS across aggressive parameter sweep. No sliver tolerance.
XOR Validation: 211 test cases, 100% pass rate
Code organization
sky130/logic/— 437 digital standard cells (sky130_fd_sc_hd__*)sky130/fixed/— 290 fixed analog/RF primitives (imported GDS)sky130/pcells/— 21 parametric generators across 6 family filesTest results
scripts/magic/Dockerfile)🤖 Generated with Claude Code