feat(cspdk): add logical electrical pins for SPICE export - #334
Draft
das-dias wants to merge 4 commits into
Draft
Conversation
Add add_electric_pins() to all PCells with electrical ports. Logical-only mode. Closes doplaydo/pdks#45
…egistration - Bump gdsfactory 9.45→9.48.0 - Delete _common.py wrapper - Remove add_electrical_pins calls from cells wrapping upstream gf.c.* components that already register pins (wire_corner, wire_corner45, wire_corner45_straight, taper_metal, pad, compass) - Keep add_electrical_pins for cells wrapping gf.c.straight, gf.c.bend_circular, gf.c.bend_s which don't register pins upstream - Rewrite test_electrical_pins.py with EXPECTED_PIN_NAMES for all 6 variants - Regenerate GDS/settings/model reference files for gdsfactory 9.48
das-dias
requested review from
MarkCunningham0410,
cdaunt,
joamatab and
thanojo
as code owners
August 20, 2026 17:15
Reviewer's GuideAdds logical electrical pin registration for specific metal routing cells, upgrades gdsfactory to 9.48, introduces a parametrized test suite to verify electrical pins for six PDK variants, and refreshes reference settings/GDS fixtures to match the new upstream behavior. Sequence diagram for electrical pin registration in metal waveguide cellssequenceDiagram
participant Caller
participant straight_metal
participant bend_metal
participant bend_s_metal
participant gf_c as gf.c
participant add_electrical_pins
Caller->>straight_metal: straight_metal(length, cross_section, width)
straight_metal->>gf_c: straight(length, cross_section, width, npoints)
gf_c-->>straight_metal: Component c
straight_metal->>add_electrical_pins: add_electrical_pins(c)
add_electrical_pins-->>straight_metal: c with electrical pins
straight_metal-->>Caller: Component c
Caller->>bend_metal: bend_metal(angle, cross_section, width)
bend_metal->>gf_c: bend_circular(radius, angle, width, cross_section, npoints, layer)
gf_c-->>bend_metal: Component c
bend_metal->>add_electrical_pins: add_electrical_pins(c)
add_electrical_pins-->>bend_metal: c with electrical pins
bend_metal-->>Caller: Component c
Caller->>bend_s_metal: bend_s_metal(size, cross_section, width, allow_min_radius_violation)
bend_s_metal->>gf_c: bend_s(size, cross_section, npoints, allow_min_radius_violation, width)
gf_c-->>bend_s_metal: Component c
bend_s_metal->>add_electrical_pins: add_electrical_pins(c)
add_electrical_pins-->>bend_s_metal: c with electrical pins
bend_s_metal-->>Caller: Component c
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}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The electrical pin tests in
tests/test_electrical_pins.pyrepeat the same three patterns for each PDK; consider parametrizing over(PDK, EXPECTED_PIN_NAMES)to remove the duplicated test blocks and keep them in sync more easily. - In the waveguide cells where you wrap
gf.c.straight/bend_circular/bend_s, a small helper (e.g.straight_with_pins,bend_with_pins) could reduce repetition ofc = ...; add_electrical_pins(c); return cand make it harder to forget pin registration in future cells.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The electrical pin tests in `tests/test_electrical_pins.py` repeat the same three patterns for each PDK; consider parametrizing over `(PDK, EXPECTED_PIN_NAMES)` to remove the duplicated test blocks and keep them in sync more easily.
- In the waveguide cells where you wrap `gf.c.straight`/`bend_circular`/`bend_s`, a small helper (e.g. `straight_with_pins`, `bend_with_pins`) could reduce repetition of `c = ...; add_electrical_pins(c); return c` and make it harder to forget pin registration in future cells.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
das-dias
marked this pull request as draft
August 21, 2026 08:34
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
1 participant
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.
Closes doplaydo/pdks#45
Summary
~=9.48.0_common.pywrapperadd_electrical_pinscalls from cells wrapping upstreamgf.c.*components that already register pins in 9.48 (wire_corner, wire_corner45, wire_corner45_straight, taper_metal, pad, compass)add_electrical_pinson cells usinggf.c.straight/bend_circular/bend_s(which don't register pins internally)EXPECTED_PIN_NAMES+ 120 parametrized tests across all 6 variants (si220_oband, si220_cband, si340, sin200, sin300, si500)Test plan
Summary by Sourcery
Enable reliable logical electrical pin export for SPICE across the supported PDK cells.
New Features:
Enhancements:
Build:
Tests:
Chores: