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

Change definition of FluxTime and DFT fields with complex fields #1279

Merged
merged 1 commit into from
Dec 10, 2023

Conversation

caseyflex
Copy link
Contributor

@caseyflex caseyflex commented Dec 1, 2023

This PR:

  • Changes FluxTime and DFT calculation to only use real part of fields
  • Updates sim.complex_fields based on nonlinear models

@momchil-flex momchil-flex self-requested a review December 1, 2023 18:17
Copy link
Collaborator

@momchil-flex momchil-flex left a comment

Choose a reason for hiding this comment

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

Could you add a changelog item explaining the change?

I guess let's merge this once we're 100% clear on the modifications we want to do, including freq domain.

@momchil-flex
Copy link
Collaborator

So in the current state, the following would be happening:

  • FieldTimeMonitor returns complex fields just in case, but we make it clear that the real fields would typically be the ones of interest, and the imaginary fields are not used in many derived quantities.
  • FieldMonitor returns fields that are the DFT of Real(E(t))
  • FluxTimeMonitor returns flux based on Real(E(t)) only; FieldTimeMonitor.flux is also correspondingly modified.
  • Time and frequency domain fluxes are then consistent with one another and the total power is independent of whether real or complex fields are used.

@caseyflex caseyflex marked this pull request as draft December 5, 2023 19:21
@caseyflex caseyflex self-assigned this Dec 6, 2023
@caseyflex caseyflex marked this pull request as ready for review December 6, 2023 13:29
@caseyflex caseyflex changed the title Changed time-domain flux with complex fields to only use real part Change definition of FluxTime and DFT fields with complex fields Dec 6, 2023
@caseyflex caseyflex force-pushed the casey/fluxtimenormalization branch 2 times, most recently from beb2593 to 5488ffd Compare December 7, 2023 14:15
CHANGELOG.md Outdated Show resolved Hide resolved
tidy3d/components/data/sim_data.py Outdated Show resolved Hide resolved
tidy3d/components/medium.py Outdated Show resolved Hide resolved
tidy3d/components/time.py Outdated Show resolved Hide resolved
@caseyflex caseyflex force-pushed the casey/fluxtimenormalization branch 2 times, most recently from 546c173 to de083b2 Compare December 8, 2023 18:53
@momchil-flex momchil-flex merged commit 259d1c4 into pre/2.5 Dec 10, 2023
14 checks passed
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.

None yet

2 participants