gdstk backend: close API gaps so the tutorials run - #104
Conversation
|
Great catch. Thanks! |
|
Can you please fix the merge conflicts? |
6da8d59 to
2677d02
Compare
|
Rebased onto current Two commits were dropped rather than carried over: both touched On the snap-to-grid commit in particular, one measurement that may be worth knowing. The DRC/LVS workflows run the gdsfactory backend, so gdstk is not covered by CI. Building the gf180 cells on gdstk with #102 applied gets them to build, but DRC then reports off-grid geometry: That is the class this commit removes: |
|
One more thing on the relationship with #102, since this branch is the approved one and will likely land first. The two conflict in
Verified with that resolution on top of
No behaviour change against the tree those were first measured on. Detail left here as well as on #102 so it is visible from whichever side is merged second — I can rebase #102 once this one is in. |
|
Pushed one more commit here:
That was the last gap stopping the tutorial suite from running on gdstk. Measured on this fork's CI, with this PR plus #102 and #113 applied:
This commit touches only the backend, so it does not change what this PR's own CI exercises — the notebooks job here runs gdsfactory either way. |
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.
Running the tutorial notebooks under GLAYOUT_BACKEND=gdstk fails almost immediately: 4 of 14 pass. Two reasons, one on each side. The notebooks import Component, rectangle, boolean and cell straight from gdsfactory instead of glayout.backend, so they bypass the backend selection entirely. Under the default backend both names resolve to the same class and nobody notices; under gdstk, gdsfactory's add_ref() gets a gdstk Component and rejects it. Symbols glayout.backend does not re-export (text_freetype, array) are left on gdsfactory. The gdstk ComponentReference/Component are also missing pieces of the gdsfactory surface that glayout's own cells and the tutorials use: movex/movey took a bare delta; move() already accepted destination= ref.name read-only, but cells label placements (ref.name = "pfet_2") ref.x / ref.y had xmin/xmax/ymin/ymax but not the centre accessors write_gds required a filename and ignored gdsdir= Component.show absent ref.name stores the label on the reference rather than renaming the target cell, which would rename every other placement of it too. Tutorial notebooks under gdstk: 4/14 -> 10/14. The three BJT tutorials still fail (Component indexing, add_ref(columns=)) and are untouched here. Notebooks under the default gdsfactory backend are unaffected: the imports resolve to the same objects.
Every vertex of a gdstk-generated layout lands off-grid on gf180. The DRC
reports it on all of them -- 1301 violations on the LIF cell used to check
this, split across contact_OFFGRID x348, via1_OFFGRID x276,
metal1_OFFGRID x252, metal2_OFFGRID x132 and comp_OFFGRID x60.
snap_to_grid() took a bare `nm: int = 1` default. gdsfactory's version reads
the pitch from the active PDK instead:
nm = int(get_grid_size() * 1000 * grid_factor)
which is 5 nm on gf180. Rounding a 5 nm process to 1 nm produces values like
10.246 where the process wants 10.245, and every one of them is a violation.
Two pieces were missing. Pdk.activate() was a no-op, so nothing recorded
which PDK was active; and grid_size kept the class default of 0.001 because
gdsfactory used to fill it in from its own PDK database on activate. The
real pitch is already in gds_write_settings.precision (5e-9 m on both gf180
and sky130), so activate() now derives grid_size from it and registers the
PDK for snap_to_grid to read. `nm=` still overrides when a caller wants a
specific pitch.
After: 0 of 1228 vertices off-grid, same as the gdsfactory backend, and the
1301 OFFGRID violations are gone. snap_to_2xgrid(10.2463) returns 10.25 on
both backends now. DRC on diff_pair, current_mirror_nfet and
transmission_gate under gdsfactory is unchanged.
move(destination=) translated relative to the bbox centre instead of (0,0), which is gdsfactory's default. c_route places its extension rectangles with move(destination=...) followed by relative movex, so the route shifted by half a rectangle: the LIF neuron came out 32.4 um wide instead of 29.9, with 54 extra M2.2a violations. get_ports_list returned dict order; gdsfactory sorts clockwise (west, north, east, south). Cells look ports up by substring, so the order changes which port gets routed. With both, neurona.ipynb gives 1168.6 um2 and 31 violations on either backend, identical to gdsfactory.
Two gaps that show up running the team's primitives notebook. add_ports iterated its argument assuming a sequence of Port, but gdsfactory also accepts a name->port mapping and glayout passes ref.ports straight in. Iterating that yields the names, so resistor() died with "'str' object has no attribute 'name'". And __getitem__ was missing. bjt indexes the component to read a port's width, and without it the error is "'Component' object is not subscriptable", which says nothing about the port it was after. Added on Component and ComponentReference, with the KeyError listing the ports that do exist. With these, resistor, pnp and npn build on the gdstk backend.
`test_bjt_gdsfactory` lays its contact rings out with `add_ref(reference, columns=, rows=, spacing=)`, which gdsfactory accepts and this backend did not, so the notebook died on a TypeError partway through. gdstk.Reference has the same notion natively, so the array stays one reference rather than becoming rows*columns of them. Verified in CI on the fork: with this, the tutorial suite runs 14/14 on gdstk. Without it, 13/14 -- that notebook the only failure.
5ebee1d to
e6144e0
Compare
2 participants
Running the tutorial notebooks under
GLAYOUT_BACKEND=gdstkfails almostimmediately — 4 of 14 pass. Two causes, one on each side.
The notebooks bypass the backend selection
Under the default backend this resolves to the same class as
glayout.backend.Component, so nobody notices. Under gdstk, gdsfactory'sadd_ref()receives a gdstkComponentand rejects it:Redirected
Component,rectangle,rectangular_ring,booleanandcelltoglayout.backend. Symbols it does not re-export(
text_freetype,array) stay on gdsfactory. Imports only — no cellcontent touched.
The gdstk backend is missing parts of the surface
Each of these is used by glayout's own cells or by the tutorials:
movex/moveydestination=too, likemove()already hadref.nameref.name = "pfet_2")ref.x/ref.yxmin/xmaxwrite_gdsfilenamerequired, nogdsdirComponent.showref.namestores the label on the reference rather than renaming thetarget cell — renaming the cell would rename every other placement of it.
Result
The three BJT tutorials still fail on
Componentindexing andadd_ref(columns=); those need more than a signature and are left alone.Notebooks under the default gdsfactory backend are unaffected — the
redirected imports resolve to the same objects.
Worth noting for #100:
glayout_opamp.ipynbandGLayout_Cells.ipynb,which currently time out at 180 s under gdsfactory, complete in 89 s and
53 s under gdstk.