phidl icon indicating copy to clipboard operation
phidl copied to clipboard

how can we fix phidl snapping errors?

Open joamatab opened this issue 1 year ago • 11 comments

import phidl.device_layout as pd
import phidl.geometry as pg

nm = 1e-3

c = pd.Device()
r1 = c << pg.compass(size=(1, 1))
r1.movey(0.5 * nm)

r2 = c << pg.compass(size=(1, 1))
r2.connect("E", r1.ports["W"])

c.write_gds("a.gds")

image

@tvt173 @helgeGehring @basnijholt

joamatab avatar Aug 12 '22 05:08 joamatab

the same approach we just implemented in gdsfactory should work just as well in phidl. you'll just need to figure out the best place to run it. in gdsfactory we have the concept of a decorator, so we can run the flatten_invalid_refs as a sort of cell finalization hook. in phidl, you could run it maybe at gds export? or manually, of course, after making your cell

https://github.com/gdsfactory/gdsfactory/pull/576 https://gdsfactory.github.io/gdsfactory/notebooks/08_pdk.html#PDK-decorator

tvt173 avatar Aug 12 '22 10:08 tvt173

Hi Troy,

this is not a snapping error due to non-manhattan connections

    import gdsfactory as gf
    nm = 1e-3

    c = gf.components.coupler(gap=101 * nm)
    c2 = gf.routing.add_fiber_array(c, decorator=gf.decorators.flatten_invalid_refs)
    c2.show(show_ports=True)

in this case the decorator does not fix the issue

image

image

You can also reproduce the error

    nm = 1e-3

    c = gf.Component()
    coupler = c << gf.components.coupler(gap=101 * nm)
    wg = c << gf.components.straight()
    wg.connect("o1", coupler.ports["o2"])
    c.show()

joamatab avatar Aug 12 '22 13:08 joamatab

This is a temporary fix where we snap gaps to a 2nm grid

https://github.com/gdsfactory/gdsfactory/pull/589

joamatab avatar Aug 12 '22 14:08 joamatab

hi @joamatab , comment out lines 53 and 73-75 of coupler.py image

then re-run your sample script with a line added to fix the invalid refs (manually applying the decorator)

import gdsfactory as gf
from gdsfactory.decorators import flatten_invalid_refs

nm = 1e-3

c = gf.Component()
coupler = c << gf.components.coupler(gap=101 * nm)
wg = c << gf.components.straight()
wg.connect("o1", coupler.ports["o2"])
flatten_invalid_refs(c)
c.show()

no more snapping errors 😺 image

the decorator fixes not just non-manhattan connections, but also off-grid connections (at least it should). still not totally sure why your calls to absorb were causing issues. but if you remove this unnecessary extra step, seems like it should work!

tvt173 avatar Aug 13 '22 03:08 tvt173

Thank you Troy,

What do you think of the snap to 2nm grid that we added to fix those issues on gaps?

joamatab avatar Aug 13 '22 03:08 joamatab

at the end of the day, the geometries will need to snap eventually. in your approach, you snap pre-emptively. in my approach, i let everything be drawn, then snap at the last minute (let's call it Just-In-Time snapping, i suppose 😄) . if you are usually placing that device at manhattan angles and on-grid points (which I assume is the case), your approach is probably preferrable. i think my approach is better i.e. for things with non-90 degree ports and maybe cascaded devices which risk accumulated error. in your approach, you will force all ports to be on-grid, which will allow devices connected to it to be placed normally, at on-grid points. in my approach, ports will be off-grid, connections will also happen off-grid, and we will need to make sure the decorator is applied across the board to prevent snapping errors

tvt173 avatar Aug 13 '22 04:08 tvt173

Unless I'm mistaken, these types of rounding errors are entirely unavoidable when working with floating point (as in gdspy/phidl)->integer conversions (as in the .gds format). Changing the amount of rounding doesn't help, because there's always 2 numbers arbitrarily close together that will round to different integer values.

Put another way: If you give me a grid-snapping/rounding value (e.g. 1nm, 2nm), I can always come up with two points that are extremely close together, but will round to different values

  • 1nm rounding: .499999e-9 / 0.500001e-9 rounds to 0e-9 / 1e-9
  • 2nm rounding: .999999e-9 / 1.00001e-9 rounds to 0e-9 / 2e-9
  • etc

This is compounded by the fact that references are not fixed to integer translations (note even if they're fixed to integers, if you allow non-manhattan rotations you will have the same problem). In a similar example, let's pretend I have two points on the x-axis, at locations 0.0000 and 0.0002, and then I put them in a reference and move them

  • Move the reference by 0.499999e-9 -> new points at .499999e-9 / 0.500001e-9 -> snap to 1nm rounds to 0e-9 / 1e-9

The only real way to fix this in the general case is to flatten ALL the points so that references don't exist, then go hunting for points which are fairly close together but differ by some tolerance. So for example, you take all the x-values in your entire flattened cell, and it gives you a list like this:

[19, -5, .01, 0.5000001, 0.5000002, 13, 0.4999999]

then you sort them and it gives you a list like this:

[-5, .01, 0.4999999, 0.5000001, 0.5000002, 13, 19]

then you do a np.diff and compare adjacent values, which gives you this:

[5.01000000e+00, 4.89999900e-01, 2.00000000e-07, 1.00000000e-07, 1.24999998e+01, 6.00000000e+00]

then you find which numbers are too close together, under some tolerance (say, 1e-6) and merge those numbers to a single value, taking you from this:

[19, -5, .01, 0.5000001, 0.5000002, 13, 0.4999999]

to this:

[19, -5, .01, 0.4999999, 0.4999999, 13, 0.4999999]

Which no longer can have any rounding errors. There's actually an implementation of this in geometry.py called _merge_floating_point_errors() / _merge_nearby_floating_points()

Note that if you maintain references with non-integer values, you end up with the still having the same problem because now your references can accidentally round incorrectly (e.g. reference 1 has a translation of 0.49999, and reference 2 has a translation of 0.50001)!

amccaugh avatar Aug 31 '22 16:08 amccaugh

hi @amccaugh , on a high level i agree with you, but in practice i find that the vast majority of snapping errors happen under one of two conditions

  1. References have origins at off-grid (floating point value) locations
  2. References are rotated at non-90 degree rotations

In either case, values for these reference origins/rotations will be cast to an eight-byte real number when written to GDS, and then interpreted as an on-grid point. This creates a loss in precision which will be inconsistent between different references.

In the vast majority of cases, simply flattening the cell resolves these issues, because then, if all math leading up to that point is consistent, two values (i.e. two ports which should be connected) end up at exactly, or near-exactly the same point, and they will undergo the same loss in precision after rounding (now because they are within the same reference).

The decorator I made, which I was mentioning above goes just a slight step beyond that... instead of flattening the whole cell (the owner of all "bad" references), I flatten just the individual references into new cells with 0 origin and 0 rotation, in an attempt to preserve hierarchy.

I agree that yes, there is still some chance if having inconsistent math down the line that leaves two points which should overlap at slightly different locations which get "unfortunately rounded", I think this is a low-probability phenomenon though, and I find that flattening, via one of the two methods above fixes most snapping errors.

tvt173 avatar Aug 31 '22 21:08 tvt173

see https://gdsfactory.github.io/gdsfactory/notebooks/08_pdk.html#PDK-decorator

joamatab avatar Aug 31 '22 23:08 joamatab

That's an interesting point, I see how it could be bad that you're essentially doing a 2-stage rounding process when you convert local polygons and then references to integer units. Do you have any test cases I might be able to play around with? In particular, I'd be curious if you don't need to flatten and center at (0,0) -- if instead you can get away with just rounding the references origins to some precision. So if a reference had the origin (0, 1.2345678), you "snap" it to e.g (0, 1.23456) or (0, 1.23). Obviously this wouldn't help with arbitrary rotations, but it might help significantly with reference-translation errors for minimal effort.

I'm also amenable to build in a Device.snap() function that performs the same operation as your PDK decorator. It seems like useful functionality

amccaugh avatar Sep 01 '22 18:09 amccaugh

hi @amccaugh , yes in short that does, work from my experimentation. our first iteration did something similar to that. you have to be careful about a few things though... i.e. that the user may go through a whole sequence of move, mirror, rotate operations, you should make sure you are doing this at the very end. and because of the combination of things that can happen with mirror + rotation, etc. I found it was simplest and most straightforward to just flatten the reference at the very end rather than implement too much new logic. of course some hierarchy is lost this way though. it would be nice to have a way to do this which preserves full hierarchy, but I think that gets quite a bit more complicated

tvt173 avatar Sep 01 '22 18:09 tvt173