Correct collision algorithm in RZ. #2510

Yin-YinjianZhao · 2021-10-28T16:48:18Z

In this PR, we will try to do a particle momentum rotation before calling the collision function, such that different azimuth positions of particles are also taken into account.

… collision_rz

Yin-YinjianZhao · 2021-11-05T22:22:33Z

input.txt
This input file is used to test the PR.
With this PR, collisions should not change particle velocities.
Without this PR, collisions will change particle velocities randomly.

As shown in the plot, 1, 2, ..., 5 are five sample particles casually picked among total 640 particles.
All particles are tested using this script.
script.txt
It compares all particle velocities over time and show that all their velocities are not changed.

BTW, a small bug is fixed in UpdateMomentumPerezElastic.H.
I added a relative difference judgement at the beginning.
I.e., if the two particle's velocities are too close, no need to collide.
Previous judgement is too strict, and may lead to nan, such that the test mentioned before fails.

lgtm-com · 2021-11-08T17:41:12Z

This pull request introduces 2 alerts when merging bb5a147 into 13f6e98 - view on LGTM.com

new alerts:

1 for Unused import
1 for Variable defined multiple times

Examples/Tests/collision/inputs_rz

lgtm-com · 2021-11-09T21:53:15Z

This pull request introduces 2 alerts when merging 9453118 into 9341e09 - view on LGTM.com

new alerts:

1 for Unused import
1 for Variable defined multiple times

Source/Particles/Collision/BinaryCollision/ElasticCollisionPerez.H

ax3l

Great work! Tiny suggestions inline.

Examples/Tests/collision/analysis_collision_rz.py

Regression/WarpX-tests.ini

Source/Particles/Collision/BinaryCollision/ElasticCollisionPerez.H

Co-authored-by: Axel Huebl <[email protected]>

Examples/Tests/collision/analysis_collision_rz.py

RemiLehe · 2021-11-23T17:44:35Z

As suggested by @NeilZaim, the generalization for higher-order modes could be implemented by binning particles in theta.

PhilMiller · 2021-11-24T06:37:47Z

Source/Particles/Collision/BinaryCollision/ElasticCollisionPerez.H

+          u1x[I1[i1]] = u1xbuf*std::cos(theta) - u1y[I1[i1]]*std::sin(theta);
+          u1y[I1[i1]] = u1xbuf*std::sin(theta) + u1y[I1[i1]]*std::cos(theta);
+#endif
+


Why write to the backing arrays rather than just using local variables for these two values? This seems like pointless memory traffic.

Ok, so I see these are passed by reference, and updated inside UpdateMomentumPerezElastic. There's still an opportunity to do that write into a stack variable that won't be flushed to memory, and then only write it out once it's potentially been corrected by the following de-rotation.

@PhilMiller Hi, thanks for you comments, could you provide more details how to improve the code, I'm not sure if I understand your point. Stack variables also work on GPUs?

Yin-YinjianZhao#2
Here's a PR against your branch with my suggestion

RemiLehe

Thanks for this PR. I added a request for the assert statement to be moved to another part of the code.

Docs/source/usage/parameters.rst

Examples/Tests/collision/analysis_collision_rz.py

Source/Particles/Collision/BinaryCollision/ElasticCollisionPerez.H

* Add rotation. * Fix a small bug * Add automated test. * rename inputs_rz * Add json * minor * add comment. * Apply suggestions from code review Co-authored-by: Axel Huebl <[email protected]> * Update analysis_collision_rz.py * Fix missing import * Change tolerance. * Add warnings * Apply suggestions from code review * Move assert. * fix end-of-Line whitespaces. Co-authored-by: Axel Huebl <[email protected]> Co-authored-by: Remi Lehe <[email protected]>

Add rotation.

e5a29ad

Yin-YinjianZhao requested a review from RemiLehe October 28, 2021 16:48

Yin-YinjianZhao added the component: collisions Anything related to particle collisions label Oct 28, 2021

Yin-YinjianZhao changed the title ~~Correct collision algorithm in RZ.~~ [WIP] Correct collision algorithm in RZ. Oct 28, 2021

Yin-YinjianZhao added 3 commits November 1, 2021 09:00

Merge branch 'development' of https://github.com/ECP-WarpX/WarpX into…

d86b048

… collision_rz

Merge branch 'development' of https://github.com/ECP-WarpX/WarpX into…

a9bf51f

… collision_rz

Fix a small bug

c659885

Yin-YinjianZhao added 3 commits November 8, 2021 09:10

merge

1d2483d

Add automated test.

bbc8b5b

rename inputs_rz

bb5a147

ax3l reviewed Nov 8, 2021

View reviewed changes

Examples/Tests/collision/inputs_rz Outdated Show resolved Hide resolved

ax3l reviewed Nov 8, 2021

View reviewed changes

Examples/Tests/collision/inputs_rz Outdated Show resolved Hide resolved

ax3l added the geometry: RZ axisymmetric 2D and quasi-3D label Nov 8, 2021

RemiLehe mentioned this pull request Nov 9, 2021

Correct binary fusion algorithm in RZ #2539

Open

Add json

9453118

Yin-YinjianZhao changed the title ~~[WIP] Correct collision algorithm in RZ.~~ Correct collision algorithm in RZ. Nov 9, 2021

minor

74a47d1

dpgrote reviewed Nov 10, 2021

View reviewed changes

Source/Particles/Collision/BinaryCollision/ElasticCollisionPerez.H Show resolved Hide resolved

add comment.

17b7465

ax3l reviewed Nov 11, 2021

View reviewed changes

Yin-YinjianZhao and others added 2 commits November 11, 2021 10:26

Apply suggestions from code review

c234306

Co-authored-by: Axel Huebl <[email protected]>

Update analysis_collision_rz.py

411d21f

ax3l reviewed Nov 16, 2021

View reviewed changes

Examples/Tests/collision/analysis_collision_rz.py Show resolved Hide resolved

Fix missing import

73892dd

ax3l assigned RemiLehe Nov 16, 2021

Yin-YinjianZhao added 2 commits November 16, 2021 11:11

Change tolerance.

c5c9d82

merge

787f5de

Add warnings

306dd0c

Yin-YinjianZhao mentioned this pull request Nov 16, 2021

[WIP] Correct fusion algorithm in RZ #2569

Closed

PhilMiller reviewed Nov 24, 2021

View reviewed changes

RemiLehe requested changes Dec 2, 2021

View reviewed changes

RemiLehe and others added 3 commits December 2, 2021 09:07

Apply suggestions from code review

2b81a12

Move assert.

ab21e68

fix end-of-Line whitespaces.

61df218

RemiLehe approved these changes Dec 6, 2021

View reviewed changes

RemiLehe merged commit 713f5ae into BLAST-WarpX:development Dec 6, 2021

RemiLehe mentioned this pull request Sep 19, 2022

Correct and test fusion module in RZ geometry #3255

Merged

Yin-YinjianZhao deleted the collision_rz branch September 21, 2022 20:43

Correct collision algorithm in RZ. #2510

Correct collision algorithm in RZ. #2510

Uh oh!

Conversation

Yin-YinjianZhao commented Oct 28, 2021

Uh oh!

Yin-YinjianZhao commented Nov 5, 2021

Uh oh!

lgtm-com bot commented Nov 8, 2021

Uh oh!

Uh oh!

Uh oh!

lgtm-com bot commented Nov 9, 2021

Uh oh!

Uh oh!

ax3l left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RemiLehe commented Nov 23, 2021

Uh oh!

PhilMiller Nov 24, 2021

Choose a reason for hiding this comment

Uh oh!

PhilMiller Nov 24, 2021

Choose a reason for hiding this comment

Uh oh!

Yin-YinjianZhao Dec 2, 2021

Choose a reason for hiding this comment

Uh oh!

PhilMiller Dec 3, 2021

Choose a reason for hiding this comment

Uh oh!

RemiLehe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants