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

inverse design with dispersive metallic antenna #124

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

tylerflex
Copy link
Collaborator

@tylerflex tylerflex commented Jun 14, 2024

Requires flexcompute/tidy3d#1761 for CustomPoleResidue support in autograd.

Also the results aren't great yet.

image

Copy link
Contributor

@e-g-melo e-g-melo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @tylerflex! The new inverse design plugin seems very exciting!

Some comments about the notebook:

  1. Maybe @tomflexcompute could create a nice image for this example?
  2. I suggest to make this sentence more clear. "This uses the new autograd support in tidy3d >= 2.7.0 but the same differentiation support with respect to td.CustomPoleResidue is not supported in tidy3d's adjoint plugin.".
  3. Merge cells [2] and [3].
  4. It seems there are some comments in the cell [8] you need to remove. Like "# doesnt work" and "# medium_gold = td.material_library['SiN']['Horiba']"
  5. Cell numbers are missing after cell [11].
  6. It would be nice to introduce and give more details on the invdes plugin and the functions make_filter_and_project, and get_kernel_size_px after cell [11]. Or maybe include more details about the new inverse design architecture using autograd in the introduction. So, the existing Adjoint users could easily understand the differences between the new approach and the older one using jax.
  7. Some typos:
  • "the desity value" -> "the density value"
  1. It seems isbox is imported but not used. If that is not the case, we could have some comments about it.
  2. Should we show the initial antenna geometry in the xy plot?
    image
  3. I didn't find the definition for sim_antenna_random.
  4. There are some empty cells along the notebook.

@tylerflex tylerflex force-pushed the tyler/autograd_/antenna branch from 847f967 to 4355137 Compare July 3, 2024 23:32
@tylerflex
Copy link
Collaborator Author

@e-g-melo I incoporated your suggestions, thanks!

@tomflexcompute would you mind taking a look at this updated notebook? the results are much better now.

@tomflexcompute
Copy link
Contributor

@e-g-melo I incoporated your suggestions, thanks!

@tomflexcompute would you mind taking a look at this updated notebook? the results are much better now.

Thanks @tylerflex . I remember leaving comments on both PRs but not sure why it's gone on this one. Anyway happy to read over the notebook again.

Copy link
Contributor

@e-g-melo e-g-melo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @tylerflex! It is a very interesting example!

@tylerflex tylerflex force-pushed the tyler/autograd_/antenna branch from 4355137 to dfee69d Compare July 4, 2024 21:03
@tylerflex tylerflex merged commit a6e6607 into develop Jul 4, 2024
@tylerflex tylerflex deleted the tyler/autograd_/antenna branch July 4, 2024 21:05
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

Successfully merging this pull request may close these issues.

3 participants