search

flexcompute / tidy3d

Fast electromagnetic solver (FDTD) at scale.
https://www.flexcompute.com/tidy3d/solver/
GNU Lesser General Public License v2.1
162 stars 41 forks source link

Elliptical Cylinder Geometry #1682

Open tylerflex opened 1 month ago

momchil-flex commented 1 month ago

I think maybe instead of a separate Geometry class this could be a Cylinder classmethod that creates a transformed cylinder?

tylerflex commented 1 month ago

I had the same idea. @e-g-melo mentioned it would be better for gui integration if we made it its own class. At the same time, we don't want to clutter the geometry primitives so I'm not sure

momchil-flex commented 1 month ago

The classmethod approach would also be easier for the backend, as in, no changes needed, vs. needing to implement an Ellipse there too otherwise. But at the end of the day maybe we should do whatever makes the GUI experience best.

tylerflex commented 1 month ago

What would be nice is a way for GuI to offer ways to enter fields using class method constructors. But then the original data is lost when we load the model back

e-g-melo commented 1 month ago

A native Ellipse would be ideal for GUI, but that is not super urgent. Two academic users have asked for that, so if you decide to implement it, we could have this as a lower-priority task.

tylerflex commented 1 month ago

maybe we can offer a classmethod for now? should be a super simple task and potentially good one for new people to implement to get familiar with the code.

e-g-melo commented 1 month ago

I think so. It will certainly add. It would be nice if we could make it so that users easily understand they are getting a transformed object.

tylerflex commented 1 month ago

It will be in the type annotation shown in the docs

def elliptical(...) -> Transformed:
    ...

however, it might be a circular import if Transformed ever imports Cylinder, so perhaps we can't technically do this. It might rather need to be a class method of Transformed.