libs.tech: Fix klayout extraction of diodes #225
Conversation
Diodes were extracted at the wrong hierarchy level. Matthias (KLayout author) was kind enough to debug this issue and came up with this patch. See related discussion : https://www.klayout.de/forum/discussion/2605/device-extracted-at-wrong-level-of-hierarchy#latest But as a summary, KLayout's booleans use the first argument's hierarchy as reference. so it's important to make sure to use a "cell local" layer/polygon as first argument to ensure extraction at the proper level Signed-off-by: Sylvain Munaut <tnt@246tNt.com>
smunaut
requested review from
KrzysztofHerman and
sergeiandreyev
as code owners
|
Hi @atorkmabrains, @FaragElsayed2, could you please review this PR? |
|
Can someone re-run the check ? It was just a timeout fetching klayout ... |
There was a problem hiding this comment.
@smunaut I'm not sure if that change would fix the issue in general or not. Because it depends on the specific test case provided. If we change the order of the layer and have the "activ" inside the the layer and antenna_d_mk outside and large marker. The hierarchy will be corrupted as well.
Basically, I think the issue is within Klayout engine itself. It should be able to push all devices down the hierarchy based on the netlist provided. As long as the all the device layers exist, It shouldn't matter which layer is top and which is bottom.
|
|
||
| # ==== dpantenna diode ==== | ||
| dpantenna_p = pactiv.and(antenna_d_mk) | ||
| dpantenna_n = nwell_drw.covering(dpantenna_p) |
There was a problem hiding this comment.
@sergeiandreyev @smunaut I don't agree with that change.
|
Yes, you can create some weird hierarchy that would break this rule and still be extracted wrong. And ATM the standard cell library, which uses the sane way of drawing diodes is broken and this patch fixes it. Could KLayout be better ? Sure, but you guys chose it as your LVS engine and according to its author (which we can reasonable assume knows his stuff), this is the best way to implement the diode LVS extraction ... |
|
@smunaut Having one of the recognition layers on top of the series of devices happens if you have a large array of devices for example and it's kind of common practice on that side when people draw a massive array of devices. I'm not arguing about how the layout is done or where the diode should be placed in the hierarchy, that's not part of my concern, but we as rule deck developers, we have to handle all possible scenarios the user might through at us fundamentally, regardless where the layer is placed in the hierarchy. In LVS, there is no such thing called "Recog Layer" or "hierarchy layer". The LVS engine (Either in Open Source or Commercial Tools) looks for a device "Identification" layer which is usually a derived layer of a combination of multiple layers. In "Commercial Tools", it continues further the analysis and check where it should place the device in the hierarchy to perfectly match the input. Hierarchy manipulation in Klayout needs some updates to make it function better and more reliably. BTW, on a side note, I believe Matthias is a great guy, for me he's a hero, he is doing this all aside his main job which is extremely incredible. We choose Klayout as it was the best "Open Source" LVS tool out there. It's intuitive and much easier to use than Magic. Matthias has recommended this change based on the unique example that you have provided and most likely he has analyzed the rule deck hierarchy against that change and saw the hierarchy breaks. I believe it can't be generalized to make this pass as it would be case by case. Now going to the "standard cells" and we should follow, we were not responsible of that part. That's a call @sergeiandreyev and @KrzysztofHerman has to do not me. As far as LVS goes, if it passes LVS regression, I would say it's up to you to decide if you want to incorporate that or not. And for my comment that I don't agree with the change is using I'll try to rerun actions now if it passes. |
|
@sergeiandreyev @KrzysztofHerman I don't have the permissions to rerun actions, Could you please do that for us? |
|
@atorkmabrains I have just triggered the workflow |
Yes. But having that drawn on a different hierarchy level (i.e. in the top cell) rather than in the same cell as the array of device is what I'd find weird. Beside, in that case, It'd be pretty understandable if those were extracted in the hierarchy level where that recognition layer is drawn.
So ... it's doing exactly what you prefer. It's literally removing |
|
@KrzysztofHerman The run fails because it's not able to pull klayout for some reason. Not sure why. |
|
It seems that Matthias has put some form of traffic filter on the website as the same commands pass in all the other test cases but only fails in that one. @KrzysztofHerman Would it be possible to keep the klayout |
|
@atorkmabrains I could store the |
|
@KrzysztofHerman, the exact version I guess is: |
|
@atorkmabrains @sergeiandreyev I can setup a small http server to host the |
Diodes were extracted at the wrong hierarchy level. Matthias (KLayout author) was kind enough to debug this issue and came up with this patch.
See related discussion :
https://www.klayout.de/forum/discussion/2605/device-extracted-at-wrong-level-of-hierarchy#latest
But as a summary, KLayout's booleans use the first argument's hierarchy as reference. so it's important to make sure to use a "cell local" layer/polygon as first argument to ensure extraction at the proper level