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.

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

Explicit solver with Ion motion #737

Merged
merged 20 commits into from
May 21, 2022

Conversation

AlexanderSinn
Copy link
Member

@AlexanderSinn AlexanderSinn commented May 1, 2022

This PR adds support for ion motion to the explicit solver. For this the fields rho jx jy jz jxx jxy jyy are required for each plasma species separately. In case of plasmas.names = electrons ions the fields rho_beam jx_beam jy_beam jz_beam rho_electrons jx_electrons jy_electrons jz_electrons jxx_electrons jxy_electrons jyy_electrons rho_ions jx_ions jy_ions jz_ions jxx_ions jxy_ions jyy_ions will be allocated. This is a bit ugly in various places of the code for now, but I have plans to eliminate the need for those fields in the future by directly depositing Sx, Sy and Mult from the plasma particles. This would involve:

  • Adding a new deposition Kernel for Sx, Sy and Mult
  • Change HPMG to support ghost cells of Sx, Sy and Mult (ghost cells needed for current deposition; also possible to copy fields to somewhere without ghost cells)
  • Removing jxx jxy jyy from the deposition
  • Changing fields back to just rho jx jy jz for all plasmas combined (maybe beams+plasmas)

The behavior with predictor-corrector solver should be unchanged.

image
image

  • Small enough (< few 100s of lines), otherwise it should probably be split into smaller PRs
  • Tested (describe the tests in the PR description)
  • Runs on GPU (basic: the code compiles and run well with the new module)
  • Contains an automated test (checksum and/or comparison with theory)
  • Documented: all elements (classes and their members, functions, namespaces, etc.) are documented
  • Constified (All that can be const is const)
  • Code is clean (no unwanted comments, )
  • Style and code conventions are respected at the bottom of https://github.com/Hi-PACE/hipace
  • Proper label and GitHub project, if applicable

@AlexanderSinn AlexanderSinn added component: plasma About the plasma species component: fields About 3D fields and slices, field solvers etc. labels May 20, 2022
@AlexanderSinn AlexanderSinn requested a review from MaxThevenet May 20, 2022 12:15
Copy link
Member

@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 small inline comments. In addition:

  • Could you add a CI test with ion motion + explicit? For instance, a low-resolution simulation with electrons and "heavy" (5x mass) positrons, with (i) a benchmark for explicit and (ii) a comparison between explicit and PC. In the linear regime, we should be able to get results of decent accuracy at acceptable cost.
  • Could you briefly describe (in the PR description, or in an issue) how you envision to clean the source terms, and what impact it will have on the current implementation?

src/fields/Fields.cpp Show resolved Hide resolved
src/fields/Fields.cpp Show resolved Hide resolved
src/fields/Fields.H Show resolved Hide resolved
src/particles/MultiPlasma.cpp Show resolved Hide resolved
src/particles/PlasmaParticleContainer.H Outdated Show resolved Hide resolved
src/Hipace.cpp Outdated Show resolved Hide resolved
src/Hipace.cpp Outdated Show resolved Hide resolved
@AlexanderSinn
Copy link
Member Author

I added a CI test comparing explicit solver against predictor-corrector with ion motion which correctly fails if (one of):

  • the beam tilt is removed
  • the beam transverse momentum is removed
  • the ion mass is changed from 5*m_e to 4*m_e or 6*m_e

in the explicit simulation.

@AlexanderSinn AlexanderSinn requested a review from MaxThevenet May 21, 2022 14:56
Copy link
Member

@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.

Cool, thanks for the thorough test! And you managed to make it quick enough, that's very nice. I have one last point: ion_motion.SI.1Rank.sh has quite a collection of command-line parameters, so the input file differs quite a bit from linear_wake/inputs_SI. Is there a reason to use that input file rather than creating a new folder e.g. examples/ion_motion?

@@ -0,0 +1,44 @@
#! /usr/bin/env python3
Copy link
Member

Choose a reason for hiding this comment

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

This is the script we discussed that compares 2 openPMD files and checks whether they contain the same data to some precision, right? Indeed this is nice and useful. Let's keep it in linear_wake for the moment as you suggest, and if we start using it a lot in the future we'll put it somewhere else (and add a few input options like --field-var-list, --species-list, particle-var-list, --tolerance, stuff like that).

Copy link
Member Author

@AlexanderSinn AlexanderSinn May 21, 2022

Choose a reason for hiding this comment

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

Not really, I had something way more general in mind, independent of coordinate and unit systems as well as automatic plotting of differences. This was just something quick.

Copy link
Member Author

Choose a reason for hiding this comment

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

Would a new folder be better than just a new inputs_ion_motion_SI file in linear_wake?

Copy link
Member

Choose a reason for hiding this comment

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

You're right, new input file is probably better.

Copy link
Member

@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 to me, we can merge as soon as tests pass. Thanks again!

@MaxThevenet MaxThevenet merged commit 05df262 into Hi-PACE:development May 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: fields About 3D fields and slices, field solvers etc. component: plasma About the plasma species
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants