TeamAtomECS / AtomECS

Cold atom simulation code
GNU General Public License v3.0
46 stars 12 forks source link

Change line 235 in gaussian.rs under laser, resolve energy not conser… #93

Open YijunTangCambridge opened 2 months ago

YijunTangCambridge commented 2 months ago

Identified an error in gaussian.rs line 235, where I found the calculation of vector there is not correct. When ellipticity is defined in gaussian.rs approach by redefining x and y the calculation of intensity gradient will have to include extra factor dx'/dx as in the attached photo.

This issue is spotted when I investigate energy dynamic of a single atom, check kinetic energy, potential energy and total energy, found out that energy is not conserved for non-zero ellipticity. After imposing the changes to line 235 I redo energy conservation check, energy are conserved, comparison are shown in the other photon.

elliptic

elliptic2
ElliotB256 commented 2 months ago

Thanks for the report, I'll double check this but it looks good at first glance!

ElliotB256 commented 2 months ago

If possible, could you define a unit test that fails for the earlier broken code and passes for your correct code? That would help to improve the coverage and prevent it occuring again.

YijunTangCambridge commented 2 months ago

Sounds good, I will look up what tests were there already and see what I can do.

YijunTangCambridge commented 2 months ago

Hi Elliot, I have now made modification on existing unit tests (there are 3 wrong unit test when I test the clean version, all about dipole force), and I added a new one to test the ellipticity issue, now there are no error from my end, could you check and proceed to approval if good? Many thanks :)

YijunTangCambridge commented 2 months ago

1) Correct unit test: laser::gaussian::tests: test_get_gaussian_beam_intensity_gradient

2) Added and tested correct new unit test: test_get_elliptic gaussian_beam_intensity_gradient

3) Correct unit test: laser::intensity_gradient::tests::test_sample_laser_intensity_gradient_again_system

4) Correct unit test: dipole::force::tests::test_apply_dipole_force_and_gradient_system

I just summarize the changes I made

YijunTangCambridge commented 2 months ago

Hi Elliot, just a reminder about my ellipticity fix :)