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

A fix to prevent division by zero during normalization #239

Merged
merged 3 commits into from
Jul 15, 2024

Conversation

hightower8083
Copy link
Member

I have recently added import_from_lasy and export_to_lasy methods in axiprop and for the latter one I have used an empty profile to get it's field rewritten, i.e.:

wavelength = 2 * np.pi * c / Container.omega0
empty_longitudinal_profile = LongitudinalProfile(wavelength=wavelength)
empty_transverse_profile = TransverseProfile()
empty_profile = CombinedLongitudinalTransverseProfile(
    wavelength=wavelength,
    pol=polarization,
    laser_energy=0.0,
    long_profile=empty_longitudinal_profile,
    trans_profile=empty_transverse_profile,
)

With this I've noticed that the normalization forced at initialization of CombinedLongitudinalTransverseProfile divides field by 0.0, and generates the corresponding warning. Although my methods work fine with this (None field values are overwritten anyway), I don't like this warning.

So this PR checks the energy before normalization and skips it if field is zero.

@ax3l ax3l added the bug Something isn't working label Apr 30, 2024
Copy link
Member

@ax3l ax3l left a comment

Choose a reason for hiding this comment

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

LGTM - thanks a lot!

@ax3l ax3l requested a review from RemiLehe April 30, 2024 19:59
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.

IIRC, we (in particular @RemiLehe) discussed offline and suggested that the check could be in Laser.normalize, or maybe in each normalize_... in utils. I don't remember what we concluded, but IMO the latter is good so the check is really performed where needed, regardless on how we wrap or call the normalize_.... That way, we shouldn't need to change laser.py.

@RemiLehe RemiLehe requested a review from MaxThevenet July 15, 2024 06:00
@RemiLehe
Copy link
Member

I updated the PR according to the above comment.

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 good, thanks!

@MaxThevenet MaxThevenet merged commit e493952 into LASY-org:development Jul 15, 2024
6 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants