Add support for geometry transformations #1203
Conversation
lucas-flexcompute
requested review from
tylerflex,
shashwat-sh and
weiliangjin2021
There was a problem hiding this comment.
@lucas-flexcompute this is really comprehensive and will be great to have. Thanks a lot. I jumped in to take a look before noticing this was a draft, so sorry if any of the comments were things that you just hadn't got to yet. Just a few initial thoughts as I read through things.
|
Very cool feature! Overall looks good. |
lucas-flexcompute
force-pushed
the
lucas/geometry_transformations
branch
2 times, most recently
from
a686776
to
c9a41d7
Compare
lucas-flexcompute
requested review from
shashwat-sh,
weiliangjin2021 and
tylerflex
There was a problem hiding this comment.
Thanks @lucas-flexcompute, I just have a set of minor comments / suggestions. Everything related to the functionality looks really good to me! I'm approving for when the comments are addressed / discussed.
|
Because of the discussion about the back-end processing, transformed slanted polyslabs should not be allowed, but if we add the validator in |
lucas-flexcompute
force-pushed
the
lucas/geometry_transformations
branch
3 times, most recently
from
a734d61
to
b7e45cb
Compare
| _ = np.linalg.inv(val) | ||
| return val | ||
|
|
||
| @pydantic.root_validator(skip_on_failure=True) |
There was a problem hiding this comment.
What's the reason for adding skip_on_failure=True here?
There was a problem hiding this comment.
I doesn't make much sense to use this validator if geometry or transform aren't really one of the expected types or the transform is singular. It'll just raise another error. Isn't that what skip_on_failure means?
lucas-flexcompute
force-pushed
the
lucas/geometry_transformations
branch
from
04471c3
to
f096d44
Compare
tidy3d/components/structure.py
Outdated
| """Prevents the creation of slanted polyslabs rotated out of plane.""" | ||
| if ( | ||
| isinstance(val, Transformed) | ||
| and isinstance(val.geometry, PolySlab) |
There was a problem hiding this comment.
what if a geometrygroup or clipper operator is transformed, and it contains a slanted polyslab? is it included here already?
There was a problem hiding this comment.
Good catch, I guess not. We'll need a recursive check in this case. Let me think it through and change it.
Support is added through a new `Geometry` subclass: `Transformed` and a few convenience functions to instance it from other geometries. Related issues: - flexcompute/tidy3d-core#25 - flexcompute/tidy3d-core#339 Signed-off-by: Lucas Heitzmann Gabrielli <lucas@flexcompute.com>
lucas-flexcompute
force-pushed
the
lucas/geometry_transformations
branch
from
f096d44
to
c440c80
Compare
Based on flexcompute/tidy3d#1203 Signed-off-by: Lucas Heitzmann Gabrielli <lucas@flexcompute.com>
Based on flexcompute/tidy3d#1203 Signed-off-by: Lucas Heitzmann Gabrielli <lucas@flexcompute.com>
Support is added through a new
Geometrysubclass:Transformedand a few convenience functions to instance it from other geometries.Related issues: