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

Crash when deleting a hierarchical PCell library from Python #905

Closed
iqmtestd opened this issue Sep 13, 2021 · 5 comments
Closed

Crash when deleting a hierarchical PCell library from Python #905

iqmtestd opened this issue Sep 13, 2021 · 5 comments
Assignees
Milestone

Comments

@iqmtestd
Copy link

Hi @klayoutmatthias,

Thanks for making KLayout! It works great with our KQcircuits project to design quantum computers.

We have several libraries of PCells in the above Python package. Everything works fine but I had to disable reloading libraries as it is crashing with KLayout 0.27. By reloading I mean deleting the library and registering it anew.

Specifically, when deleting a library that has PCells that use PCells from the same library we get :
ERROR: ../../../src/db/db/dbLayout.cc,1692,topological_sort ()

This problem is very similar to an earlier bug: #646. Luckily, I could even use the same example code (with small changes) to reproduce the problem. If pcell2 is not using pcell1 then then it works. If I remove the tl_assert from line 1692 then everything is peachy but I'm not certain that this is the right solution...

import pya

class PCell1(pya.PCellDeclarationHelper):

  def produce_impl(self):
    pass

  def display_text_impl(self):
    return "PCell1"

class PCell2(pya.PCellDeclarationHelper):

  def produce_impl(self):
    cell = self.layout.create_cell("PCell1", "PCell1Library", {})
    self.cell.insert(pya.DCellInstArray(cell.cell_index(), pya.DTrans()))

  def display_text_impl(self):
    return "PCell2"

def create_libraries():
  library1 = pya.Library()
  library1.description = "Test library"
  library1.layout().register_pcell("PCell1", PCell1())
  library1.layout().register_pcell("PCell2", PCell2())
  library1.register("PCell1Library")

def reload_libraries():
  mw = pya.MainWindow.instance()
  library1 = pya.Library.library_by_name("PCell1Library")
  library1.delete()
  create_libraries()
  mw.create_layout(1)

def create_pcell():
  cell_view = pya.CellView.active()
  layout = cell_view.layout()
  cell = layout.create_cell("PCell2", "PCell1Library", {})
  cell_view.cell = cell

pya.MainWindow.instance().create_layout(1)
create_libraries()
create_pcell()
reload_libraries()

Best regards,
David

@lukasc-ubc
Copy link

I've had similar issues for many years in the SiEPIC-Tools package, namely crashing when reloading libraries. Matthias has advised me not to have PCells instantiating other PCells. Rather he suggests separating out the drawing code into separate functions. I personally would love if we could just have hierarchical PCell support.

@klayoutmatthias
Copy link
Collaborator

@lqmtestd Many thanks for this well-prepared test case. I can reproduce the problem and hopefully fix it.

Lukas is right - initially, calling PCells from other PCells was not quite stable. It should be well supported now, but deleting or exchanging libraries is a tricky operations. KLayout needs to update many references and trigger PCell recomputation and it's likely that in some cases some references are not properly updated. I assume this happens mainly during development - in production mode, libraries should not change while you use them.

Kind regards,

Matthias

klayoutmatthias added a commit that referenced this issue Sep 14, 2021
@klayoutmatthias
Copy link
Collaborator

@iqmtestd I was able to fix the problem.

The reason is that inside the PCell2 layout generation you explicitly refer to "PCell1Library". This is not wrong, but this is actually an external reference which points back to the library the PCell2 is defined in. So when you delete "PCell1Library", this external reference needs to be handled properly as it will point to the library which is being destroyed.

You can see from the name shown in the cell tree that there is an additional reference. The "PCell1Library" shows up twice in the instantiation path:

image

An easy workaround is to drop this external reference:

class PCell2(pya.PCellDeclarationHelper):

  def produce_impl(self):
    cell = self.layout.create_cell("PCell1", {})
    self.cell.insert(pya.DCellInstArray(cell.cell_index(), pya.DTrans()))

  def display_text_impl(self):
    return "PCell2"

This works, because KLayout evaluates the PCell generation code in the context of the library, not the target layout. By using the unspecific version of "create_cell", the PCell is looked up in the current library.

Matthias

@iqmtestd
Copy link
Author

iqmtestd commented Sep 16, 2021

@klayoutmatthias thanks for the easy workaround. I've tested it with KQCircuits too and it also works there.

To be more precise, both the workaround and the bug-fix does the trick: no more crashes! The workaround has the additional benefit of removing the duplicate library references in the cell tree. See the FluxlineStandard pcell before and after library reload:

image

An interesting but harmless side-effect of reload is the appearance of temporary "ghost" cells in the cell tree.

@klayoutmatthias
Copy link
Collaborator

klayoutmatthias commented Sep 17, 2021

@iqmtestd Very good! Thanks for this feedback :)

Usually KLayout cleans up idle proxy cells, but maybe not on reload. I'll see on occasion if that can be polished, but so far I agree it should not cause any harm.

Matthias

klayoutmatthias added a commit that referenced this issue Sep 21, 2021
Fixed #905 (Crash while deleting a Library)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants