FLYonPIC v2.0

[BrianMarre]

Second version of FLYonPIC now includes:

• electronic collisional de-/excitation
• spontaneous deexcitation
• electronic collisional ionization
• autonomous ionization
• Stewart-Pyatt ionization potential depression(IPD)
• pressure ionization based on the IPD implementation

as well as a reworked atomic rate solver with adaptive time stepping

still missing:

• [x] FLYonPIC input specification
• [x] compile test
• [x] user facing description

requires

• [x] PR #4918 to be merged first
• [x] PR #4944 to be merged first

[BrianMarre]

@PrometheusPi and @steindev for now no review required unless you really want to, a detailed examination together with @psychocoderHPC is already planned

[BrianMarre]

@ikbuibui @chillenzer it is now at a state where a review from you would be helpful

[chillenzer]

Wow! So, how many months do we get for 20k+ lines of code?

[BrianMarre]

@psychocoderHPC do you have any idea why the CI fails at the KHI test/example? I am scratching my head on that one.

[BrianMarre]

@psychocoderHPC please restart the KHI CI job, failed again with message "KILLED"

[BrianMarre]

@psychocoderHPC @chillenzer @ikbuibui @PrometheusPi @steindev ready for review

[BrianMarre]

@psychocoderHPC @PrometheusPi CI fails in the KHI_growth rate test with,

[ 78%] Built target picongpu-hostonly
[ 80%] Building CUDA object CMakeFiles/picongpu.dir/main.cpp.o
Killed
make[2]: *** [CMakeFiles/picongpu.dir/build.make:76: CMakeFiles/picongpu.dir/main.cpp.o] Error 137
make[1]: *** [CMakeFiles/Makefile2:243: CMakeFiles/picongpu.dir/all] Error 2
make: *** [Makefile:136: all] Error 2

I am not sure how to avoid this, I think psychocoderHPC suggested that this may be due to the CI running out of memory, but I am unsure why this would be specific to the FLYonPIC PR, other PRs CI tests pass fine and atomicPhysics should not be active at all in the KHI_growth test.

In summary my guess is that I am doing something wrong but I am unsure what exactly. Any help would be appreciated.

[PrometheusPi]

If the single KHI_growthrate test job fails again, we have an issue with 64bit precision for that specific setup. This would require a detailed study, since this would then reveal unexpected side effects.

[chillenzer]

Hi, here's a first batch of comments. These are more or less general comments, more detailed code comments as well as comments concerning the rest of the files will follow. In general, I will tag the comments, etc. with one of the following pre-fixes:

REQUEST: Necessary change (or at least necessary discussion).

REFACTOR: Nice-to-have but given the circumstances, we should use the strategy from here and create an issue with such comments to address them later.

CONSCIOUS DECISION: A decision that has a trade-off. By commenting on this, I want to provoke a conscious decision that this is the best trade-off. You can resolve immediately (or with a short reasoning) if you think you thought this through.

QUESTION: More for my understanding than to entail code changes.

Please excuse that these are abbreviated to the edge of being impolite. They are copied more or less directly from my even shorter notes.

Comments:

• CONSCIOUS DECISION: atomicPhysics_Debug.param
  • It surprised me that they exist and are different in the tests folder:
    • Some of the settings are backend-specific. Does this render the tests backend-specific?
    • Wouldn't we want to test the production version and not some debug version that might behave differently? (If only by different scheduling in multi-threading.)
    • The tests are compilation-only, so why activate debug there (assuming it's runtime debug)?
• REQUEST: Tests are missing READMEs.
• REFACTOR: An example would be nice:
  • Or are the tests also valid physical cases?
  • If so, having an example with more context and explanation would probably still be beneficial.
• CONSCIOUS DECISION: atomicPhysics.param is identical between default and tests/compileAtomicPhysics. Should we remove the latter?
• REQEUST: Is the FLYonPIC step diagram in the docs up to date? (At some point you showed me a version of this that you said was outdated, so just checking.)
  • Where does the arrow from "binElectrons" lead to?
  • What does this KERNEL (/transitionType) mean floating in the middle of nowhere?
  • What does the true node mean?
• REQUEST: Why are there any changes in include/picongpu/fields/incidentField/profiles/DispersivePulse.hpp?
• CONSCIOUS DECISION: Why remove DOI in docs/source/usage/workflows/ionization.rst?
• REQUEST: Unnecessary and questionable changes in docs/source/models/collisional_ionization.rst.

[BrianMarre]

@chillenzer REQUEST FLYonPIC diagram:

• FLYonPIC diagram is updated
• good catch, I missed the extra floating text and too short life line for the electron histogram, fixed
• the true node indicates a while(true) , did not find a better way to draw it with PlantUML

REQUEST changes in docs/source/models/collisional_ionization.rst and include/picongpu/fields/incidentField/profiles/DispersivePulse.hpp:

• I am not sure what exactly you mean, according to Github the PR does not touch docs/source/models/collisional_ionization.rst or include/picongpu/fields/incidentField/profiles/DispersivePulse.hpp?

REFACTOR add Examples: Well, the fundamental problem is that the atomic data input files I am using are not public, therefore we are unable to provide a setup that works at runtime.

[chillenzer]

I am not sure what exactly you mean, according to Github the PR does not touch docs/source/models/collisional_ionization.rst or include/picongpu/fields/incidentField/profiles/DispersivePulse.hpp?

Okay. Then, ignore it. I pulled one specific commit last week Thursday night which I also found is outdated by now.

REFACTOR add Examples: Well, the fundamental problem is that the atomic data input files I am using are not public, therefore we are unable to provide a setup that works at runtime.

Okay. (Well, not okay philosophically and concerning open science, etc., but that's probably not something we want to solve before merging this.)

[psychocoderHPC]

The failing CI test is the KHi with double precision. It is running into the time limit of 3h.

[psychocoderHPC]

@BrianMarre please rebase against the dev branch. We removed ISAAC support, maybe this helps to pass the CI. This will reduce compile overhead, if not there is an issue with 64bit PIConGPU together with atomic physics.

[BrianMarre]

I checked that 94f1aab still fulfills the atomicPhysicsCI test

[psychocoderHPC]

@BrianMarre please add (seperate PR) the svg or source data of your big plot you added as png. The png can not be editied.