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.

Sign up for GitHub

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

Jump to bottom

Add cos- longitudinal profile #207

Merged
merged 23 commits into from
Jan 17, 2024
Merged

Add cos- longitudinal profile #207

merged 23 commits into from
Jan 17, 2024

Conversation

lucafedeli88
Copy link
Contributor

@lucafedeli88 lucafedeli88 commented Nov 6, 2023 •
edited
Loading

This PR adds a new longitudinal profile: a truncated cosine, defined essentially as:

tn = (t - self.t_peak) / self.tau_fwhm
envelope = (
    np.cos(0.5 * np.pi * tn)
    * (tn > -1)
    * (tn < 1)
    * np.exp(+1.0j * (self.cep_phase + self.omega0 * self.t_peak)
    )

I called this profile CosLongitudinalProfile, but I can rename it CosineLongitudinalProfile or TruncatedCosineLongitudinalProfile if you prefer.

17/12/2024: Upon discussion we agreed to call this profile CosineLongitudinalProfile

pre-commit-ci bot and others added 13 commits November 6, 2023 16:50
@lucafedeli88 lucafedeli88 added documentation Improvements or additions to documentation new feature request or implementation of a new feature labels Nov 8, 2023
@lucafedeli88 lucafedeli88 changed the title [WIP] Add cos- longitudinal profile Add cos- longitudinal profile Nov 8, 2023
@AngelFP AngelFP self-assigned this Jan 15, 2024
AngelFP
AngelFP requested changes Jan 17, 2024
View reviewed changes
Copy link
Member

@AngelFP AngelFP left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for this PR! Great to have some added tests. Could you check the small comment below?

lasy/profiles/longitudinal/cosine_profile.py
Comment on lines +24 to +25
tau_fwhm : float (in second)
The Full-Width-Half-Maximum duration of the intensity distribution of the pulse,
Copy link
Member

Choose a reason for hiding this comment

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

the description of the wavelength parameter is missing here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right! Thanks for having spotted this!

@lucafedeli88 lucafedeli88 requested a review from AngelFP January 17, 2024 18:14
AngelFP
AngelFP approved these changes Jan 17, 2024
View reviewed changes
Copy link
Member

@AngelFP AngelFP 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 now, thanks!

@AngelFP AngelFP merged commit 1b712df into LASY-org:development Jan 17, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AngelFP AngelFP AngelFP approved these changes

@MaxThevenet MaxThevenet Awaiting requested review from MaxThevenet

@RemiLehe RemiLehe Awaiting requested review from RemiLehe

Assignees

@AngelFP AngelFP

Labels
documentation Improvements or additions to documentation new feature request or implementation of a new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lucafedeli88 @AngelFP