Add SetOperation to construct more complex geometry #1052
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lucas-flexcompute
changed the title
lucas-flexcompute
force-pushed
the
lucas/set_operations
branch
from
c918c02
to
50816d7
Compare
tylerflex
approved these changes
Aug 7, 2023
There was a problem hiding this comment.
looks good, I can't follow many of the internals so maybe @weiliangjin2021 can comment on that. But in general I think it will be really nice to have this stuff generalized like this. Thanks @lucas-flexcompute
lucas-flexcompute
force-pushed
the
lucas/set_operations
branch
from
535a799
to
41a7793
Compare
|
With both |
weiliangjin2021
approved these changes
Aug 11, 2023
|
Great! I'll wait for any revisions required from the back-end before merging. |
Also, allow GeometryGroup to accept GeometryGroup instances as group elements. That's because ClipOperation must accept them as operands, so it makes sense to extend the support to GeometryGroup, since the backend code is already in place. Importing a general gds file requires the implementation of `ComplexPolySlab`, but the plugin implementation depends on `Medium` and `Structure` for the `to_structure` method, so it cannot be simply refactored into the geometry namespace. The solution was to have a base class without the problematic method in geometry, which can be used to import gds files. Signed-off-by: Lucas Heitzmann Gabrielli <lucas@flexcompute.com>
lucas-flexcompute
force-pushed
the
lucas/set_operations
branch
from
797309f
to
e4a1c44
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Also, allow GeometryGroup to accept GeometryGroup instances as group elements. That's because
SetOperationmust accept them as operands, so it makes sense to extend the support toGeometryGroup, since the backend code is already in place.There's some overlap between
GeometryGroupandSetOperation(operation="union")but they are left as is for a few reasons:GeometryGroupwould break backwards compatibilityGeometryGroupfor organization of structuresSetOperationwith more than 2 operands would be strange for difference and symmetric difference operationsOne option would be to have the number of operands depend on the operation, but I'm not sure that's a better options then what is currently implemented because we can't remove
GeometryGroupanyway.