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

broadband adjoint (2.5) #1192

Merged
merged 1 commit into from
Oct 11, 2023
Merged

broadband adjoint (2.5) #1192

merged 1 commit into from
Oct 11, 2023

Conversation

tylerflex
Copy link
Collaborator

@tylerflex tylerflex commented Oct 4, 2023

same as #1179 but rebased against 2.5, squashed, and polished up.

Note: works fine on version 2.4.2 but fails with version 2.5.0rc1 for some reason, looking into this.

Also note: there are two added tests (_test_adjoint_broadband ...) that should go in core.

Summary of changes:

  • users can now add multiple frequencies to output_monitors and also different output_monitors can have different frequencies.
  • internally, we maintain a sorted list of freqs_adjoint corresponding to these monitors, which become the freq0s of our adjoint sources.
  • if not set by JaxSimulation.fwidth_adjoint, the fwidth of the adjoint sources is chosen to be smaller than some factor (0.1) times the minimum difference between the freqs_adjoint. This was chosen empirically to be small enough to have low error, but not be too small. When this is set, a warning is displayed to the user outlining that an fwidth was chosen automatically, which could lead to large run_time and some mitigation approaches for tightly packed frequencies.
  • if not set by a new field JaxSimulation.run_time_adjoint, the run_time of the adjoint JaxSimulation, will be some factor (100) divided by the adjoint fwidth. Again chosen empirically to avoid early shutoff in the adjoint sims, while not being too long.
  • The adjoint E fields are left unnormalized initially, but then are retroactively normalized by the adjoint source source_time spectrum at fwidths_adjoint. This is because we need to apply a different normalization frequency-by-frequency depending on the freq0 and fwidth of the adjoint sources, which may introduce amplitude or phase differences.
  • All of the VJP calculations are vectorized over frequency. At the end of the calculations, we sum the results over the frequency axis (consistent with derivative rule: df(x,y)/dz = df/dx + dy/dy, forget the name of it.

Corresponding notebook changes: flexcompute-readthedocs/tidy3d-docs#296

I think that covers it, thanks in advance for the review!

@tylerflex tylerflex added the 2.5 label Oct 4, 2023
@tylerflex tylerflex self-assigned this Oct 4, 2023
@tylerflex tylerflex linked an issue Oct 4, 2023 that may be closed by this pull request
@tylerflex tylerflex marked this pull request as draft October 4, 2023 16:40
@tylerflex tylerflex mentioned this pull request Oct 4, 2023
4 tasks
@tylerflex tylerflex marked this pull request as ready for review October 4, 2023 16:52
@tylerflex tylerflex force-pushed the tyler/adjoint/broadband2.5 branch 3 times, most recently from 45a4c4a to e7631ea Compare October 10, 2023 14:11
tidy3d/components/source.py Outdated Show resolved Hide resolved
tidy3d/plugins/adjoint/web.py Show resolved Hide resolved
@tylerflex
Copy link
Collaborator Author

ok I just added the source thing, fixed up changelog, squashed, etc. I think this should be ready to go assuming tests all pass.

Could I leave it to you to merge the core PR? maybe with this updated submodule commit?

@momchil-flex
Copy link
Collaborator

Backend tests pass, merging everything!

@momchil-flex momchil-flex merged commit 1f655c2 into pre/2.5 Oct 11, 2023
14 checks passed
@momchil-flex momchil-flex deleted the tyler/adjoint/broadband2.5 branch October 11, 2023 23:14
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.

adjoint Multi-frequency support (with same simulation)
3 participants