Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

klayout backend #1031

Closed
joamatab opened this issue Dec 20, 2022 · 6 comments
Closed

klayout backend #1031

joamatab opened this issue Dec 20, 2022 · 6 comments
Labels
enhancement New feature or request

Comments

@joamatab
Copy link
Contributor

It would be great to have the tests passing on the klayout backend branch so we can compare speed of gdstk and klayout backend

here is a demo of the klayout backend
https://github.com/gdsfactory/kfactory

and a branch with some work in progress
https://github.com/gdsfactory/gdsfactory/tree/kfactory

advantages

  • klayout has more features than gdstk (netlist tracing, drc ...) and larger user base
  • thanks to integer based units reduces snapping errors due to rounding errors
  • reduce dependencies as we already use klayout
  • easy to install with pip (for windows, macos and centos)
  • Could support easily a klayout GUI flow

disadvantages

  • klayout API harder to use
  • klayout is a bit slower than gdstk

@sebastian-goeldi
@thomasdorch
@alexsludds
@tvt173
@flaport
@SkandanC
@heitzmann
@klayoutmatthias

@joamatab joamatab added the enhancement New feature or request label Dec 20, 2022
@heitzmann
Copy link

Of course I'm a little biased towards gdstk, :) but I'd point out that gdstk also supports pip install now (thanks to @joamatab) and I'm not sure about using integer based units. I've tried to use fixed-point arithmetic before settling to floats, but the performance was way worse (by not using specific hardware and instructions for it and increasing register pressure on the integer path). I'm honestly curious to see how the implementations compare side-by-side on complex geometries.

@sebastian-goeldi
Copy link
Collaborator

sebastian-goeldi commented Dec 23, 2022

@heitzmann Give me an example you want to test the speed for and I can write you a klayout equivalent ;).

The main reason for me to insist on integer based is this test/example: https://github.com/gdsfactory/kfactory/blob/3d22a157bf4255db56081b1fc7f6184cbd8fa3f8/tests/test_ports.py#L38 . If you have something that isn't on-grid and you do a transformation, you might round in the opposite way and then need to do fixing/merging/healing to make sure you don't have a 1 database unit gap. But if you need to do healing & fixing, might as well import the reference, flatten and add to the polygon(s).

image

@joamatab joamatab mentioned this issue Dec 29, 2022
21 tasks
@lucas-flexcompute
Copy link
Contributor

@sebastian-goeldi Agreed. But wouldn't that also happen if you used a rotation instead of just translating? This looks more of a problem of limited representation of numbers on a computer… you could just as easily treat floating points as a non-uniform grid of representable numbers. Because of the non-uniformity, you lose the exact translation (your point), but gain in accuracy (depending on the range, of course) and in speed due to modern CPU architecture. I'm not saying I'm against the move towards klayout; I'm just pointing out what I found from my experience.

Unfortunately, I don't have a large layout I could openly share due to NDAs to benchmark the speed, but gdstk has a few simple benchmarks I've used to compare with gdspy. Maybe they are enough for us to have an idea.

@sebastian-goeldi
Copy link
Collaborator

@sebastian-goeldi Agreed. But wouldn't that also happen if you used a rotation instead of just translating?

Correct. This is actually a rotation and not a translation (well, it is both, but afaik gdsfactory also does this, or at least used to do it this way ;)). And that's exactly my point. Due to the nature of having to round, if you are on the rounding border (in this case that would be the 0.5 * dbu), you can always get slight inaccuracies when applying a transformation, and then you will probably round one up and one down. In this case you have the transformation in x with -20.0005, which will get rounded to -20.001, but the waveguide is exactly 20 long and therefore you end up with a 1 nm gap.

As for testing, I just tested the floating vs integer based ones with a golden spiral. Difference is negligible (I suspect because on insertion to the actual cell, shapes get converted to integer based represenations).

(kf) sgoeldi@hyper:~/repos/kfactory/tests$ python -m timeit -r 100 -n 1 -s 'from test_spiral import test_spiral' 'test_spiral()'
1 loop, best of 100: 39 msec per loop
(kf) sgoeldi@hyper:~/repos/kfactory/tests$ python -m timeit -r 100 -n 1 -s 'from test_spiral import test_dspiral' 'test_dspiral()'
1 loop, best of 100: 39.8 msec per loop

this produces a sprial like this

image

I will try to implement a similar thing in gdstk and then compare

@joamatab
Copy link
Contributor Author

We almost have finished the kfactory (klayout backend) based evaluation for gdsfactory
see branch

How can we fix route-manhattan?
I would need some help @sebastian-goeldi @SkandanC

TODO;

  • Fix route manhattan
  • Run tests as a way to compare speed

@joamatab
Copy link
Contributor Author

joamatab commented Mar 30, 2023

Ideally we would most tests in this branch passing

https://github.com/gdsfactory/gdsfactory/tree/kfactory3

AttributeError                            Traceback (most recent call last)
File ~/gdsfactory/gdsfactory/component.py:2549
   2542     assert (
   2543         c_orig.references[1].parent.references[0].parent.name
   2544         != c_new.references[1].parent.references[0].parent.name
   2545     )
   2548 if __name__ == "__main__":
-> 2549     c = Component("parent")
   2550     c2 = Component("child")
   2551     length = 10

File ~/kfactory/src/kfactory/kcell.py:1433, in KCell.__init__(self, name, klib, kdb_cel
l, ports)
   1431 super().__init__(name=name, klib=klib, kdb_cell=kdb_cell)
   1432 self.klib.register_cell(self, allow_reregister=True)
-> 1433 self.ports: Ports = ports or Ports()
   1434 self.complex = False
   1436 if kdb_cell is not None:

AttributeError: can't set attribute 'ports'

@SkandanC
@sebastian-goeldi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants