Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Implement methods for conversion between temporal and spatial field presentations #126

Merged
merged 24 commits into from
Jun 28, 2023

Conversation

hightower8083
Copy link
Member

@hightower8083 hightower8083 commented Apr 21, 2023

This PR adds to main Laser class the methods export_to_z and import_from_z for conversion between temporal and spatial field presentations, and the test script for these methods.

Additionally this PR includes:

  • fix of the use of fft method in propagator to comply with the used envelope definition: ifft with "backward" normalization should be used to convert the field to the frequency domain
  • direct import of c from scipy.constants is using instead of scc.c
  • removed the requirement for the specific version of axiprop

hightower8083 and others added 3 commits April 20, 2023 18:24
added and tested the wrappers for z2t and t2z convertors
…tions of lasy objects

removed absorbers and useless stuff
fixed bug with the phase terms for envelopes
replaced `scc.c` with `c` for the cleared code
added docstrings to the new methods
@hightower8083 hightower8083 added the new feature request or implementation of a new feature label Apr 22, 2023
@RemiLehe RemiLehe self-requested a review April 24, 2023 16:35
Copy link
Contributor

@MaxThevenet MaxThevenet left a 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!! See comments below. Furthermore, could you add a CI test? Suggestions:

  • in 3D, for the Gaussian example that you shared in your figures, generate a beam with lasy (so, time representation), call export_to_z, and compare with theory (the tolerance will ofc depend on the resolution, but it should be tight enough for the t representation to largely fail the test). Then you could call import_from_z, and compare with the original profile.
  • Same test in RZ, as these use quite different functions.

Co-authored-by: Maxence Thévenet <maxence.thevenet@desy.de>
@MaxThevenet
Copy link
Contributor

Could you have a look at the pre-commit.ci issue?

@MaxThevenet
Copy link
Contributor

Thanks! as described in the message above, could you add a CI test for this PR? This should be consistent with what was discussed also with @RemiLehe at a LASY meeting a few weeks back.

removed unused laser object
@hightower8083
Copy link
Member Author

hightower8083 commented May 24, 2023

@MaxThevenet, @RemiLehe

I've added a simple test where we take field in t domain, export it to z domain and compare (with looser tolerance) with the original field considering approximationt=-z/c (for this I consider a large beam), then transform it back to t domain, and compare it to the original field (with higher accuracy).

I've also used for convenience a new axiprop container class which handles spectral-temporal transforms, axes and boundaries. This makes code clearer, and i'm thinking of passing to this approach in the rest of axiprop-related methods. Please take a look at the code in export_to_z and tell me what you think.

I've also removed forced version from axiprop in pip setup

Copy link
Contributor

@MaxThevenet MaxThevenet left a comment

Choose a reason for hiding this comment

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

Thanks for adding the test! See comment below.

assert np.allclose(
laser_t_in.field.field[ind0], laser_t_out.field.field[ind0], atol=1e-8
)
assert np.allclose(Field_approx, Field_orig, atol=2e-3)
Copy link
Contributor

Choose a reason for hiding this comment

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

Are Field_orig and Field_approx supposed to be close to each other in general? Otherwise, could we compare instead something like (pseudo-code) laser_t_in and laser_z*(1+z**2/zR**2)**.5 for a beam with a length = zR centered around t=0 and z=0? These should be close.

hightower8083 and others added 2 commits June 27, 2023 17:26
…ntations

defined the checks for `t-z-t` numerical correctness and analytical check for `z` representation
@hightower8083
Copy link
Member Author

Hey @MaxThevenet , I've revised the test
Now operates with a case with strong difference with t and z representations. It verify the reversibility of t-z-t transform and also compares the obtained z representation with theory.
On my machine tests runs fine, but for some reason fails in CI and I fail to understand the reason. Could you check how it runs on your side and let me know if you have any idea how to debug the unittest ?

Copy link
Contributor

@MaxThevenet MaxThevenet left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for this PR! Below are minor typos fixes.

@MaxThevenet MaxThevenet merged commit 41a7313 into LASY-org:development Jun 28, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
new feature request or implementation of a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants