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

Cell.read doesn't read LayoutMetaInfo #1609

Closed
sebastian-goeldi opened this issue Feb 2, 2024 · 9 comments · Fixed by #1613
Closed

Cell.read doesn't read LayoutMetaInfo #1609

sebastian-goeldi opened this issue Feb 2, 2024 · 9 comments · Fixed by #1613
Assignees
Labels
Milestone

Comments

@sebastian-goeldi
Copy link
Contributor

sebastian-goeldi commented Feb 2, 2024

Hello Matthias,

Hope you are doing great. I noticed that Cell.read LayoutMetaInfo of the cell. I didn't check for child cells, so this might also be intended behavior.

Currently, Layout.read will read LayoutMetaInfo for the cells. Cell.read on the other hand will not load the meta infos. Is this intended behavior? If I look at the example equivalent code in the docs, it would be clear to me that it is in fact not reading meta infos. But what isn't clear to me is:

Would this also apply to child cells and how would LoadLayoutOptions.CellConflictResolution play into this?

Also, how is that handled in the general case (Layout and Cell read)?

I would assume AddToCell and OverwriteCell would read them if a cell exists (but e.g. how would AddToCell handle existing meta info?), whereas Skip would not write any.

If this is intended behavior, can we add a few sentences about this either into the LoadLayoutOptions or the read functions?

here's my test case which will fail on Cell.read

import klayout.db as kdb

ly = kdb.Layout()
c = ly.create_cell("test_cell")
mi = kdb.LayoutMetaInfo("test", 42, "test_value", True)
c.add_meta_info(mi)

ly.write("test_ly.oas")

ly_read = kdb.Layout()
ly_read.read("test_ly.oas")

c_read = ly_read.cell("test_cell")
mi_read = c_read.meta_info(mi.name)
assert mi_read.name == mi.name
assert mi_read.value == mi.value
assert mi_read.description == mi.description
assert mi_read.persisted == mi.persisted

ly_read_cell = kdb.Layout()
llo = kdb.LoadLayoutOptions()
llo.cell_conflict_resolution = kdb.LoadLayoutOptions.CellConflictResolution.AddToCell
llo.properties_enabled = True

c_read_cell = ly_read_cell.create_cell("test_cell")
c_read_cell.read("test_ly.oas", llo)
mi_read_cell = c_read_cell.meta_info(mi.name)
assert mi_read_cell.name == mi.name # will fail because the meta is never created
assert mi_read_cell.value == mi.value
assert mi_read_cell.description == mi.description
assert mi_read_cell.persisted == mi.persisted
@klayoutmatthias
Copy link
Collaborator

Hi Sebastian,

thanks, I'm doing fine :)

I'd not say, this is intended behavior, rather "unknown territory" :). I guess the implementation can be change to read cell-specific meta data. However, reading layout-specific metadata is somewhat questionable in that case. I'd still skip that rather than trying to merge with the target layout's metadata somehow.

Matthias

@sebastian-goeldi
Copy link
Contributor Author

I, see. Then my guess was right ;).

I think that's a reasonable assumption about the layout. But would you even take the same stance for the cell itself or newly created subcells?

@klayoutmatthias
Copy link
Collaborator

My proposal was the following: cell.read(...) actually loads a layout, picks the top cell (there can be only one) and pushes the cell hierarchy from the layout into the target cell, creating a sub-hierarchy if required.

So when it does so, it could transfer the cell metadata into the target hierarchy: loaded layout's top cell metadata into target cell (the one you called 'read' on) and subcell metadata accordingly into the freshly generated ones.

I think this is kind of consistent. I'd not try to transfer the layout metadata as the layout is only read to pick a cell. Logically it's the cell we're interested in, not the "global" metadata.

Does that make sense?

Matthias

@sebastian-goeldi
Copy link
Contributor Author

Yes, that totally makes sense. That's also what I had in mind. My second part of the questions was much more directed towards, what would be the CellConflictResolution eqivalent for the meta data? I.e. what happens if there is an existing and new cell meta info? Because the default action for shape resolution AddToCell won't work for the rmeta info.

This also applies to the standard Layout.read, there both for Layout and Cells.

@klayoutmatthias
Copy link
Collaborator

I have not thought about this so far, but my first approach was to merge with overwrite - i.e. add key/values to existing metadata and overwrite if there was data with that key already.

klayoutmatthias pushed a commit that referenced this issue Feb 5, 2024
This also includes some more functions:
- Layout#merge_meta_info, Layout#copy_meta_info
- Layout#clear_all_meta_info
- Cell#merge_meta_info, Cell#copy_meta_info

In addition, meta info is merged when importing a layout from
another file (Layout/Import -> Other Layouts into current).
@klayoutmatthias klayoutmatthias self-assigned this Feb 5, 2024
@klayoutmatthias klayoutmatthias added this to the 0.28.16 milestone Feb 5, 2024
@klayoutmatthias klayoutmatthias linked a pull request Feb 5, 2024 that will close this issue
@sebastian-goeldi
Copy link
Contributor Author

sebastian-goeldi commented Feb 6, 2024

I think that's the approach I would have gone with as well. Can you confirm that wouldn't be the case for SkipNewCell and RenameCell? (I would assume this is the case already though as I imagine the decision for that is at the very beginning of reading the cell)

@klayoutmatthias
Copy link
Collaborator

Ahem ... frankly I have not checked yet how the meta info is treated in the case of conflicts.

I can tell the following:

  • cell#read will not honor the conflict resolution mode. It is always RenameCell and with the patch above meta info is copied into new cells and merged with existing cells.
  • layout#read currently does not merge meta info in case of conflicts, only for RenameCell except when ghost cells are involved. This can be changed pretty quickly to 'merge meta info along with shapes and instances'.

I understand it is important for you that meta info is also handled properly.

Q: is it important as of now to support conflict resolution modes for cell#read? This case is somewhat special as conflicts are inherent because I always read into an existing cell.

Matthias

@klayoutmatthias
Copy link
Collaborator

Support for meta info is included for layout#read now: RenameCell and Overwrite will now merge the meta info for existing cells, SkipNewCell will not. Global meta info is always merged.

I have also added some support for tests and undo/redo support - although there is no UI for meta info yet, you could spoil it by deleting a cell and doing "undo".

About supporting conflict resolution modes in cell#read I am still unsure.

Matthias

@sebastian-goeldi
Copy link
Contributor Author

Cell#read is not critical. I just put a warning message to avoid it for now. If there are other high priority items on your plate feel free to put both on the back burner.

I just wanted to know the way it works. layout#read is more than sufficient.

klayoutmatthias added a commit that referenced this issue Feb 11, 2024
* Fixed issue #1609 (Cell.read doesn't read LayoutMetaInfo)

This also includes some more functions:
- Layout#merge_meta_info, Layout#copy_meta_info
- Layout#clear_all_meta_info
- Cell#merge_meta_info, Cell#copy_meta_info

In addition, meta info is merged when importing a layout from
another file (Layout/Import -> Other Layouts into current).

* Meta info support in layout diff (for testing), implemented meta info merge for GDS and OASIS readers with special conflict resolution modes

* Undo support for meta info - this way we do not loose meta info when we delete a cell and undo

---------

Co-authored-by: Matthias Koefferlein <[email protected]>
klayoutmatthias added a commit that referenced this issue Feb 11, 2024
* Fixed issue #1609 (Cell.read doesn't read LayoutMetaInfo)

This also includes some more functions:
- Layout#merge_meta_info, Layout#copy_meta_info
- Layout#clear_all_meta_info
- Cell#merge_meta_info, Cell#copy_meta_info

In addition, meta info is merged when importing a layout from
another file (Layout/Import -> Other Layouts into current).

* Meta info support in layout diff (for testing), implemented meta info merge for GDS and OASIS readers with special conflict resolution modes

* Undo support for meta info - this way we do not loose meta info when we delete a cell and undo

---------

Co-authored-by: Matthias Koefferlein <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants