Skip to content

Conversation

momchil-flex
Copy link
Collaborator

This is needed to make the simple adjoint test pass. See also the comment in the changed file.

@momchil-flex momchil-flex requested a review from tylerflex March 4, 2022 21:58
@momchil-flex momchil-flex force-pushed the momchil/normalization branch from 39764e7 to 532e948 Compare March 4, 2022 22:00
@tylerflex
Copy link
Collaborator

What is meant by user-defined phase?

@tylerflex
Copy link
Collaborator

tylerflex commented Mar 4, 2022

Ok I think I understand, basically if we normalize including source_time.phase, then the phase we introduce in the simulation never makes any difference, which means adjoint won't work?

Other option would be to include the user-defined phase back into the adjoint calculation, since we can always apply it to the frequency-domain, complex-valued fields after the fact.

@momchilmm
Copy link
Contributor

Correct.

I think this should be done as here, because it is generally strange that the phase has no effect, and I don't see a downside.

@tylerflex tylerflex merged commit d6cf06e into develop Mar 4, 2022
@momchil-flex momchil-flex deleted the momchil/normalization branch March 17, 2022 18:07
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