-
Notifications
You must be signed in to change notification settings - Fork 24
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
Space time profile #182
base: development
Are you sure you want to change the base?
Space time profile #182
Conversation
space-time profile
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
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 for this PR!
I added a few inline comments. In addition, I have a few high-level comments:
- Would it be OK to include the parameter
b
in theGaussianProfile
and have its value be 0 by default, instead of having a separateSpaceTimeProfile
class? (This is the approach that we adopted for the built-it Gaussian profile in WarpX: https://github.com/ECP-WarpX/WarpX/blob/development/Source/Laser/LaserProfilesImpl/LaserProfileGaussian.cpp#L99 ; it would require reimplementing theGaussianProfile
differently in WarpX.) - Could you add an automated test, e.g. here? The test could for instance check for instance that the laser duration is unchanged in the presence of spatial chirp, or (even better) check that the laser frequency is indeed
x
dependent.
from .profile import Profile | ||
|
||
|
||
class SpaceTimeProfile(Profile): |
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.
Maybe this could actually be part of the GaussianProfile
laser. See other comments.
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.
I don't actually see how it could be applied with GaussianProfile. In that class it calls the temporal and spatial profiles separately, but we need access to the space and time dimensions at the same time.
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.
Yes, agreed. If we decide to have it applied with GaussianProfile, we would completely change the implementation of the GaussianProfile
(which is definitely possible): instead of calling the temporal and spatial profile separately, we would have the Python code for the gaussian profile directly in the body of the GaussianProfile
class. Does that work for you?
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.
Yes, ok with me. In fact, it would mostly entail pasting the code from this PR to the GaussianProfile class. However, I also wanted to add a pulse-front tilt parameter (in addition to the current "spatial chirp" parameter). I could do that in this PR so that it is all there, and then we could paste it.
Let me know how you want to continue.
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.
Sounds good. I think it is good to do all of the above as part of the current PR, i.e. adding spatial chirp, and moving all of this to the GaussianProfile
class.
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.
I've done these changes. There are some weird test and docs errors at the moment, but in any case I need to build a stand-alone test for the space-time cases. I will try to do it next week.
update docstring Co-authored-by: Remi Lehe <[email protected]>
Move space-time effects to Gaussian profile
for more information, see https://pre-commit.ci
Dependencies fix
Update GaussianProfile call
Update GaussianProfile call
re-order GaussianProfile arguments
for more information, see https://pre-commit.ci
re-order GaussianProfile arguments
for more information, see https://pre-commit.ci
re-order space-time arguments
remove trans_profile mention
remove long_profile mention
for more information, see https://pre-commit.ci
remove trans_profile and long_profile mentions
remove trans_profile and long_profile mentions
New function get_t_peak that calculates the average time of the intensity. Relevant for pulse-front tilt testing.
for more information, see https://pre-commit.ci
Add t_peak test
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
@RemiLehe I am slowly progressing on this. Right now I am finishing the tests for angular dispersion and spatial chirp. How can I select the field only at a given transverse position? Basically I want to test either the central frequency or t_peak given spatial chirp or angular dispersion, and for that I would like to have the temporal envelop at y=0 and x=w0. |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
omega, freq_sc_rms = get_frequency(laser_sc.grid, dim) | ||
omega0 = 2 * np.pi * c / laser_sc.wavelength | ||
# Expected central frequency, using the frequency gradient (Eq. 4 from Gu et al., Opt. Comm., 2004) | ||
freq_sc_expected = omega0 + laser_sc.w0 * laser_sc.b / ( |
Check notice
Code scanning / CodeQL
Unused local variable Note test
This adds the ability to create a pulse with space-time couplings. Spatial chirp at the focus (z=0) can be added with a real-valued parameter 'sc'. This uses the highest level 'Profile' class. An interesting test is if the propagator works properly to propagate such pulses to the beginning of a simulation box. Analytical equations could also be used to directly calculate the fields at a user-defined z-position.