Skip to content

tests/lvs: let the reference netlist decide which labels are pins, and the caller pick the gf180 MIM option - #112

Merged
msaligane merged 6 commits into
ReaLLMASIC:mainfrom
carloscl03:lvs-pin-filter
Aug 25, 2026
Merged

tests/lvs: let the reference netlist decide which labels are pins, and the caller pick the gf180 MIM option#112
msaligane merged 6 commits into
ReaLLMASIC:mainfrom
carloscl03:lvs-pin-filter

Conversation

@carloscl03

Copy link
Copy Markdown
Contributor

Cell LVS on main fails on three cells, all with the same shape:

[FAIL] diff_pair        extra top-level pin(s) in layout: SUB
[FAIL] diff_pair_ibias  extra top-level pin(s) in layout: VTAIL
[FAIL] opamp            extra top-level pin(s) in layout: VTAIL
        5 passed, 3 failed

A pin label is not a property of a cell, it is a property of how the cell is
used.
VTAIL is a top-level pin of a standalone diff_pair and an internal
net inside diff_pair_ibias. Elementary cells emit labels so they can be LVS'd
on their own, and a parent that flattens them inherits those names.

Deciding that in the generator means every composite has to suppress its
children's labels, at every level. with_pin_labels does it for diff_pair,
but the parameter stops there: diff_pair_ibias does not accept it and emits
labels of its own, so opamp cannot suppress anything. Each new composite is
one more place that has to remember, and one that forgets fails silently --
which is how these three got here.

The reference netlist already states which names are pins. This honours that
when staging inputs: labels whose text is not a port of the top .subckt are
dropped from the staged copy.

  • No cell has to cooperate, so it works at any depth and whether or not the
    parent flattened.
  • No ambient state. Relevant because @cell keys its cache on arguments: a
    context manager or env var can hand back a component built under the
    opposite setting.
  • Only the staged copy is filtered, so the GDS a cell ships keeps its labels
    and LEF/macro flows are unaffected.
  • Case-insensitive, because generators and schematics disagree on
    capitalisation in practice (vdd vs Vdd).

Dropping every label is reported as a warning rather than passed over -- that
is a naming mismatch rather than inheritance, and silently leaving the layout
with no pins turns into a confusing failure further down. With no readable
ports, nothing is dropped.

Cost, measured over nine real cells (5-12 labels, 49-4913 polygons): 0.1-1.0 ms
to filter, ~27 ms including the GDS round trip, against 1-8 s per cell of LVS.

Once this lands, with_pin_labels and the pin-label halves of #103 and #110
become unnecessary; #110 stops depending on #103 and can go in on its own.

Cell LVS on main fails on diff_pair, diff_pair_ibias and opamp, all with
the same shape: "extra top-level pin(s) in layout" for SUB and VTAIL.

A pin label is not a property of a cell, it is a property of how the cell is
used. VTAIL is a top-level pin of a standalone diff_pair and an internal net
inside diff_pair_ibias. Elementary cells emit labels so they can be LVS'd on
their own, and a parent that flattens them inherits those names.

Fixing that in the generator means every composite has to suppress its
children's labels at every level of the hierarchy. `with_pin_labels` does
this for diff_pair, but the parameter stops there: diff_pair_ibias does not
accept it and emits labels of its own, so opamp cannot suppress anything.
Each new composite is one more place that has to remember, and one that
forgets fails silently -- which is how these three got here.

The reference netlist already states which names are pins. Honour that when
staging inputs: drop labels whose text is not a port of the top `.subckt`.
No cell has to cooperate, it works at any depth and whether or not the
parent flattened, and it needs no ambient state -- relevant because `@cell`
keys its cache on arguments, so a context manager or env var can hand back a
stale component built under the opposite setting.

Only the staged copy is filtered; the GDS a cell ships keeps its labels, so
LEF and macro flows are unaffected. Matching is case-insensitive: generators
and schematics disagree on capitalisation in practice (`vdd` vs `Vdd`).

Dropping every label is reported as a warning rather than passed over: that
is not inheritance but a naming mismatch, and silently leaving the layout
with no pins would turn it into a confusing LVS failure downstream. With no
readable ports, nothing is dropped.

Measured on nine real cells, 5-12 labels over 49-4913 polygons: 0.1-1.0 ms
to filter, ~27 ms including the GDS round trip, against 1-8 s per cell of
LVS.
Pure Python -- no PDK, no klayout, no GDS toolchain -- so it runs wherever
pytest does. That matters here: the LVS workflow is triggered by
`workflow_run` off Cell DRC, which only fires on the default branch, so LVS
never runs on a pull request. A PR that fixes or breaks LVS cannot be seen
either way until after it merges, which is how main went red.

Six cases, including the two that are easy to get wrong: matching has to
ignore case, because generators and schematics disagree in practice (`vdd`
vs `Vdd`) and a case-sensitive compare would strip real pins; and an
unreadable port list must drop nothing rather than leave the layout with no
pins at all.

Both are checked by mutation: making the compare case-sensitive, or removing
the empty-ports guard, each fails exactly one test.
@carloscl03

Copy link
Copy Markdown
Contributor Author

Added a unit test for the filter — six cases, pure Python, no PDK or klayout needed.

Two of them are the ones that are easy to get wrong. Matching has to ignore case, because generators and schematics disagree in practice (vdd in the GDS against Vdd in the .subckt), and a case-sensitive compare would strip real pins. And an unreadable port list has to drop nothing, rather than leave the layout with no pins at all. Both are checked by mutation: making the compare case-sensitive, or removing the empty-ports guard, each fails exactly one test.

Worth flagging separately: the LVS check cannot run on this PR, or on any PR. lvs.yml triggers on workflow_run off Cell DRC, which only fires on the default branch, so LVS has 8 runs on main and 0 on every fork branch I looked at. The three green checks here confirm nothing was broken; they do not exercise what this PR does.

That is also why main went red the way it did — no PR could have shown it coming. Happy to follow up with a workflow that runs the existing tests/test_*.py on pull requests; there are 14 tests in the repo today that nothing executes.

The repo has 14 tests under tests/ and no workflow executes any of them.

That matters more than it looks: the DRC and LVS workflows run the
gdsfactory backend (CPython 3.10, gdsfactory 7.7), so gdstk has no CI
coverage at all today, and conftest.py pins the test suite to gdstk. This
job is the first thing that exercises it.

It also gives pull requests a check they can currently fail. lvs.yml is
triggered by `workflow_run` off Cell DRC, which only fires on the default
branch: LVS has runs on main and none on any fork branch, so a PR that fixes
or breaks LVS cannot be seen either way until after it merges.

Deliberately not `pip install -e .`: install_requires pins gdsfactory<=7.7.0
and numpy<=1.24.0, which forces CPython 3.10 and pulls the whole layout
stack. None of it is needed -- verified in a clean venv with only pytest,
gdstk, numpy, pandas, pydantic and docopt: 19 passed in 7.2 s.

tests/test_cells_layout.py is left out. It builds every cell in both
backends, takes ~85 s, and carries five documented xfails, so green there
would mean "nothing changed" rather than "everything works". Cell-build
coverage deserves its own argument, and its own PR.
setup-python's `cache: pip` looks for requirements.txt or pyproject.toml to
hash and fails the job when it finds neither. This repo ships setup.py.
carloscl03 added a commit to carloscl03/gLayout that referenced this pull request Aug 22, 2026
Pin markers were drawn on hardcoded sky130 layer numbers -- met1_pin
(67,16), met1_label (67,5), met2_pin (68,16), met2_label (68,5) -- which
mean nothing on gf180. They now come from the PDK.

They were also sized and placed so that the marker carried only the label
layer, tangent to the port's edge rather than centred on it. The extractor
then finds no conductor under the text, the net comes out unnamed, and LVS
reports the pin as missing from the layout. The marker is now sized to the
routable metal's minimum width and aligned centre-to-centre, so it lands
inside metal the router already placed.

Rebased onto current main. The `no_pin_labels()` context manager this branch
originally added is dropped: ReaLLMASIC#105 landed `with_pin_labels` on diff_pair, and
ReaLLMASIC#112 handles the general case in the LVS runner, where it needs no
cooperation from any cell and does not depend on ambient state that `@cell`
cannot see.
Listing the four files reproduces the problem this job exists to fix: a test
added later is not run until someone remembers to edit the workflow. ReaLLMASIC#100
adds tests/test_narrow_fets.py, which the explicit list would have silently
skipped.

Everything under tests/ now runs except test_cells_layout.py, which stays
out for the reason already documented above the step.
The runner hardcoded option A, so a cell drawing its MIM on option B
(met4/FuseTop/met5) extracted no capacitor at all and every MIM reported
as missing from the layout. The two options are mutually exclusive at
process level, so the deck can only be told one of them.

Adds --mim-option / $GF180_MIM_OPTION, defaulting to A as before, and
applies the option-A deck repair only when option A is selected -- the
deck's option B branch already connects all three plates.
@carloscl03 carloscl03 changed the title tests/lvs: let the reference netlist decide which labels are pins tests/lvs: let the reference netlist decide which labels are pins, and the caller pick the gf180 MIM option Aug 23, 2026
@carloscl03

Copy link
Copy Markdown
Contributor Author

Added one more commit to this branch, since it lands in the same file and the same harness.

run_lvs_klayout_gf180 hardcoded mim_option: "A", but mimcap() and mimcap_array() both default to option: str = "B". So a cell built with glayout's own defaults and run through glayout's own LVS harness extracts no capacitor at all — the deck only ever looks for one MIM stack, and the two are mutually exclusive at process level. Every MIM then reports as schematic device(s) missing from layout, with the layout perfectly correct.

The commit adds --mim-option / $GF180_MIM_OPTION, defaulting to "A" so existing behaviour is unchanged, and applies the option-A deck repair only when option A is actually selected — the deck's option B branch already connects all three plates.

Found while getting a gf180 cell drawn on option B through this harness: it went from 1 schematic device(s) missing from layout to Netlists match with nothing else changed.

The 6 unit tests on this branch still pass.

Say the word if you would rather have this as a separate PR and I will split it back out.

@msaligane

Copy link
Copy Markdown
Member

Great, thanks!

@msaligane
msaligane merged commit 84a6403 into ReaLLMASIC:main Aug 25, 2026
4 checks passed
msaligane pushed a commit that referenced this pull request Aug 25, 2026
Two notebooks sit on the 180s per-cell limit and cross it depending on
which runner the job lands on, so the check passes or fails on identical
content. `lvs-pin-filter` has four green runs and two red ones.

    GLayout_Cells   290s pass (#112) / 372s error (#113)
    glayout_opamp   136s pass (#112) / 198s error (#113)

The native backend builds these cells several times faster, which moves
them off the limit instead of raising it. Measured on this fork's CI --
same container, same runners -- with #102, #104 and #113 applied on both
sides, the only difference being this line:

                              gdsfactory        gdstk
    GLayout_Cells          187.8s  ERROR      14.2s  pass
    glayout_opamp          126.4s  pass       19.2s  pass
    5T_OTA_part2            90.8s  pass       16.2s  pass
    test_bjt_custom_pattern 68.2s  pass       11.4s  pass
    whole suite            660s 13/14        158s 14/14

Note the baseline already carries all three PRs and still fails: they are
what lets the notebooks run on gdstk at all, not what fixes the timeouts.

Depends on #102, #104 and #113. Without #113 `GLayout_Cells` raises in
`mimcap.py`; without #102 it raises in `Component.add`; without #104's
`add_ref(columns=)` the BJT notebook raises. This check stays red until
all three land.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants