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

Reorganize tidy3d mode and mode overlap computations #1056

Closed
momchil-flex opened this issue Dec 30, 2022 · 2 comments
Closed

Reorganize tidy3d mode and mode overlap computations #1056

momchil-flex opened this issue Dec 30, 2022 · 2 comments

Comments

@momchil-flex
Copy link
Contributor

In Tidy3D 1.8.0 we introduced FieldData.dot(other_data) and ModeSolverData.dot(other_data) methods to correctly compute overlaps between two sets of mode data from the mode solver or field data from the FDTD solver. One thing that this does better compared to the current implementation in the gdsfactory Tidy3D plugin is that the fields are spatially colocated to the same positions (the centers of the Yee grid), while in the plugin the raw fields are used, which live on the Yee grid.

This return should then use the natively supported dot method. Also, modes are now normalized coming out of the solver, so this is not needed (the abs value can be put e.g. in the return statement if this is entirely removed, e.g. return r, np.abs(integral)**2.

However, looking through the tidy3d mode plugin, I see that it utilizes our low-level compute_modes function directly, instead of our mode solver. The fields that come out of those are not packaged in a ModeSolverData and they are not normalized, so this would be more work than I thought. I see two possible approaches:

  • Rewrite the gdsfactory plugin to use the tidy3d ModeSolver directly rather than the lower-level compute_modes function. If needed, we could add a way for the ModeSolver to use arrays of n, k as opposed to structures + a mode plane, but I don't see why this would be needed. If the Tidy3D solver plugin doesn't need to use those, it should be possible to use the mode solver just through structure definitions too. So I think this is the preferred approach.
  • Keep the current compute_modes usage but create a ModeSolverData with normalized modes out of the output fields. This would seem to require porting some of the tidy3d ModeSolver code to the plugin here, which doesn't make that much sense to me.

@joamatab has mentioned that this loss computation seems too good to be true, hopefully this will fix this but it will be good to see.

@lucas-flexcompute maybe you could have a look at this when you find some time?

@lucas-flexcompute
Copy link
Contributor

@momchil-flex Sure! I'll start looking into it asap!

lucas-flexcompute added a commit to lucas-flexcompute/gdsfactory that referenced this issue Apr 14, 2023
Almost all features from the previous solver are available in some form.
We're missing the implementation of the index dictionary that was
overlaid in the previous version to eanble the user to set a
inhomogeneous index profile over the geometry. That can be enabled
through CustomMedium in the future.

There are a few improvements from the previous version, most notably the
use of a non-uniform grid.

This commit solves issues from both
gdsfactory#1056 and
flexcompute/tidy3d#652.

Signed-off-by: Lucas Heitzmann Gabrielli <lucas@flexcompute.com>
lucas-flexcompute added a commit to lucas-flexcompute/gdsfactory that referenced this issue Apr 14, 2023
Almost all features from the previous solver are available in some form.
We're missing the implementation of the index dictionary that was
overlaid in the previous version to enable the user to set a
non homogeneous index profile over the geometry. That can be enabled
through `CustomMedium` in the future.

There are a few improvements from the previous version, most notably the
use of a non-uniform grid.

This commit solves issues from both
gdsfactory#1056 and
flexcompute/tidy3d#652.

Signed-off-by: Lucas Heitzmann Gabrielli <lucas@flexcompute.com>
lucas-flexcompute added a commit to lucas-flexcompute/gdsfactory that referenced this issue Apr 14, 2023
Almost all features from the previous solver are available in some form.
We're missing the implementation of the index dictionary that was
overlaid in the previous version to enable the user to set a
non homogeneous index profile over the geometry. That can be enabled
through `CustomMedium` in the future.

There are a few improvements from the previous version, most notably the
use of a non-uniform grid.

This commit solves issues from both
gdsfactory#1056 and
flexcompute/tidy3d#652.

Signed-off-by: Lucas Heitzmann Gabrielli <lucas@flexcompute.com>
lucas-flexcompute added a commit to lucas-flexcompute/gdsfactory that referenced this issue Apr 14, 2023
Almost all features from the previous solver are available in some form.
We're missing the implementation of the index dictionary that was
overlaid in the previous version to enable the user to set a
non homogeneous index profile over the geometry. That can be enabled
through `CustomMedium` in the future.

There are a few improvements from the previous version, most notably the
use of a non-uniform grid.

This commit solves issues from both
gdsfactory#1056 and
flexcompute/tidy3d#652.

Signed-off-by: Lucas Heitzmann Gabrielli <lucas@flexcompute.com>
@joamatab
Copy link
Contributor

moved this to gplugins
gdsfactory/gplugins#138

@yaugenst

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants