-
Notifications
You must be signed in to change notification settings - Fork 41
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
Option to allow DC component in GaussianPulse spectrum #1054
Conversation
Yeah, it modifies the shape of the spectrum a bit too but is specifically
designed to have this effect I believe
…On Fri, Aug 4, 2023 at 3:02 PM Tyler Hughes ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In tidy3d/components/source.py
<#1054 (comment)>:
> def amp_time(self, time: float) -> complex:
"""Complex-valued source amplitude as a function of time."""
twidth = 1.0 / (2 * np.pi * self.fwidth)
omega0 = 2 * np.pi * self.freq0
time_shifted = time - self.offset * twidth
- const = 1j + time_shifted / twidth**2 / omega0
+ if self.allow_dc_component:
remind me again how this works when self.allow_dc_component is False
multiplying by 1j + time_shifted / twidth**2 / omega0 i guess is
equivalent to subtracting out the f=0 component from the source time?
—
Reply to this email directly, view it on GitHub
<#1054 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A3KLECKFNQXI27JPZUPRTGLXTT6HNANCNFSM6AAAAAA3EDRNWU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some pretty minor suggested changes but overall looks good!
deaae1b
to
4c6b82f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @caseyflex looks good to me, just make sure to edit the changelog too with the "remove" convention.
I'd almost rather leave the changelog as it is, since the actual functionality which was added was the ability to leave in the DC component |
Yea good point, might just suggest putting how to do that in the changelog, eg
or something. |
4c6b82f
to
471233f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks for the implementation!
471233f
to
317adc6
Compare
I introduced an extra factor of 1j in the case where we do not remove the DC component. This extra factor makes it agree with the case where we do remove the DC component in the limit of large omega0. |
Per this issue:
#690