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

Correct ion charge deposition #156

Merged
merged 15 commits into from
Sep 9, 2020
Merged

Correct ion charge deposition #156

merged 15 commits into from
Sep 9, 2020

Conversation

SeverinDiederichs
Copy link
Member

@SeverinDiederichs SeverinDiederichs commented Sep 7, 2020

This PR introduces a new way to deposit the ion charge. It resolves #122.
It depends on #155! This PR can only be merged after #155 is merged. If this one is merged first, it makes #155 redundant and it can be closed.

Previously, a uniform ion charge was assumed and simply added during the plasma current deposition.
Now, the number of slices was increased to 5, with the 5th slice being reserved for the ion charge density.

The current deposition was modified to be able to deposit the charge directly to the ion rho slice.

It was made more modular by adding boolean options which quantities to deposit (Jx + Jy, Jz, rho).
This has the advantage, that the parameter WhichSlice::Next, This, RhoIons now only affects in which slice something is deposited and nothing else (previously, it under the hood also managed what to deposit).

This PR showed a bug with the treatment of the boundaries.
Although, the currents should be exchanged with SumBoundary, it gives the wrong result (a residual current on the edges, leading to numerical noise). Therefore, in this PR FillBoundary is added for the exchange of the currents. This needs to be addressed in a separate issue.

The benchmarks had to be updated, because spurious current deposition from the previous implementation was updated. Comparison of the lower box of rho from the following input script (the y axis label should be x, not rho)
inputs_normalized.txt
Old:
rho_old
With this PR:
rho_new

I do realize that rho in the slice_deposition benchmark was changed by many orders of magnitude, but this is correct. I tested it for a linear plasma wake with slice deposition and host-device and it worked fine. I would like to point out that Bz decreases by many orders of magnitude and it should be 0 for symmetric problems.

  • 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

@SeverinDiederichs SeverinDiederichs added component: plasma About the plasma species cleaning Code cleaning, avoid duplication, better naming, better style etc. labels Sep 7, 2020
@SeverinDiederichs SeverinDiederichs changed the title Ion deposition Correct ion charge deposition Sep 7, 2020
@SeverinDiederichs SeverinDiederichs changed the title Correct ion charge deposition [WIP] Correct ion charge deposition Sep 7, 2020
@SeverinDiederichs SeverinDiederichs changed the title [WIP] Correct ion charge deposition Correct ion charge deposition Sep 7, 2020
@SeverinDiederichs SeverinDiederichs changed the title Correct ion charge deposition [WIP] Correct ion charge deposition Sep 9, 2020
@SeverinDiederichs SeverinDiederichs changed the title [WIP] Correct ion charge deposition Correct ion charge deposition Sep 9, 2020
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 good to me, I must say I didn't check all calls to DepositCurrent with the new bool arguments.

@MaxThevenet MaxThevenet merged commit acd1cda into development Sep 9, 2020
@MaxThevenet MaxThevenet deleted the ion_deposition branch September 9, 2020 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleaning Code cleaning, avoid duplication, better naming, better style etc. component: plasma About the plasma species
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Charge deposition for ions
2 participants