feat(via_stack): directional NSEW ports, case-insensitive input, degenerate single-layer#197
Conversation
…nerate single-layer support
Three changes to via_stack to support GDSFactory+ Livewire electrical routing:
1. **Directional ports**: Replace center-positioned `bottom`/`top` ports with
`{layer}_{N|S|E|W}` ports at bbox edges. N/S ports span the x-extent,
E/W ports span the y-extent. This gives the router directional information
for multi-layer electrical routes.
2. **Case-insensitive input**: Normalize `bottom_layer`/`top_layer` against
known BEOL names so callers can pass connectivity-style names (e.g.
`METAL1`, `metal1`) and they resolve to the canonical `Metal1` used
internally. Original input casing is preserved in port labels.
3. **Degenerate single-layer case**: Allow `bottom_layer == top_layer`.
Produces a single metal rectangle with NSEW ports (no vias). This lets
the build pipeline use via_stack uniformly for both multi-layer and
same-layer junction points without special-casing.
Reviewer's GuideAdds directional NSEW electrical ports to via_stack, normalizes layer name inputs case-insensitively, and supports a degenerate single-layer case without error while preserving original labels in port names. 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 left some high level feedback:
- In the degenerate single-layer case, when
bottom_layerandtop_layerdiffer only by case (e.g.METAL1vsMetal1), the current_port_layerscondition still adds two sets of ports on the same physical layer; if the intent is a single set of{layer}_{N,S,E,W}ports, consider tightening the condition to only add one logical layer of ports after normalization. - You might want to reuse the normalized layer names (
bottom_layer/top_layer) consistently for port labels as well, or else clearly separate the concepts (e.g.normalized_layervsconnectivity_label) to avoid confusion where ports on the same physical layer have different naming conventions depending on input casing.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the degenerate single-layer case, when `bottom_layer` and `top_layer` differ only by case (e.g. `METAL1` vs `Metal1`), the current `_port_layers` condition still adds two sets of ports on the same physical layer; if the intent is a single set of `{layer}_{N,S,E,W}` ports, consider tightening the condition to only add one logical layer of ports after normalization.
- You might want to reuse the normalized layer names (`bottom_layer`/`top_layer`) consistently for port labels as well, or else clearly separate the concepts (e.g. `normalized_layer` vs `connectivity_label`) to avoid confusion where ports on the same physical layer have different naming conventions depending on input casing.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request introduces case-insensitive normalization for layer names in via_stack and replaces the single "bottom" and "top" ports with directional ports (N, S, E, W) at the bounding box edges for both layers. It also relaxes the layer order check to allow the bottom and top layers to be identical. The review feedback correctly identifies a bug where duplicate overlapping ports could be created if the bottom and top layers normalize to the same physical layer but have different casings, and provides a code suggestion to resolve this.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| _port_layers = [(bottom_layer, bottom_port_label)] | ||
| if top_layer != bottom_layer or top_port_label != bottom_port_label: | ||
| _port_layers.append((top_layer, top_port_label)) |
There was a problem hiding this comment.
When bottom_layer and top_layer normalize to the same physical layer but have different casings (e.g., "metal1" and "Metal1"), the condition top_port_label != bottom_port_label evaluates to True. This causes duplicate overlapping ports to be added on the same physical layer, which can lead to routing errors in gdsfactory. We should only append the top layer ports if the normalized layers are physically different.
| _port_layers = [(bottom_layer, bottom_port_label)] | |
| if top_layer != bottom_layer or top_port_label != bottom_port_label: | |
| _port_layers.append((top_layer, top_port_label)) | |
| _port_layers = [(bottom_layer, bottom_port_label)] | |
| if top_layer != bottom_layer: | |
| _port_layers.append((top_layer, top_port_label)) |
Port names changed from bottom/top to {layer}_{N|S|E|W} — update the
regression reference to match the new port scheme.
Summary
Three changes to
via_stackto support GDSFactory+ Livewire electrical routing:bottom/topports with{layer}_{N|S|E|W}ports at bbox edges. N/S ports span the x-extent, E/W ports span the y-extent — gives the router directional routing information for multi-layer electrical routes.bottom_layer/top_layerare matched case-insensitively against known BEOL names (e.g.METAL1→Metal1). Original input casing is preserved in port labels so callers can pass connectivity-style names.bottom_layer == top_layernow produces a single metal rectangle with NSEW ports instead of raisingValueError. This lets the build pipeline usevia_stackuniformly for both cross-layer and same-layer junction points.Breaking change
Port names change from
bottom/topto{layer}_{N|S|E|W}(e.g.Metal1_N,Metal2_E). Checked all callers in-tree (capacitors.py,sample_to_3d_electrical.py, LNA examples) — none reference ports by name, they only use the component as a positioned ref.Test plan
via_stack("Metal1", "Metal2")producesMetal1_{N,S,E,W}andMetal2_{N,S,E,W}ports at correct bbox positionsvia_stack("METAL1", "METAL2")resolves case and creates correct geometryvia_stack("Metal1", "Metal1")produces single-layer rectangle withMetal1_{N,S,E,W}ports, no viasvia_stack("Metal2", "Metal1")still raisesValueErrorcapacitors.pyMOM cap) unaffected — CI should confirmSummary by Sourcery
Update via_stack to support directional NSEW electrical ports with case-insensitive layer handling and a single-layer degenerate case.
New Features:
Bug Fixes:
Enhancements: