mabarnes / moment_kinetics

Other
2 stars 4 forks source link

Cross species collisions #200

Closed mrhardman closed 2 months ago

mrhardman commented 2 months ago

Addition of a test collision operator that consists of the self collision operator, plus cross-species collisions with two fixed Maxwellian species, representing slow ions and electrons. Example input files are added. The distributions functions shown in the attached .gif files show the relaxation of the distribution function towards cold ash. Comments on the outstanding tasks below would be especially appreciated, as these have to do with the input parameters for collision operators generally.

(@LucasMontoya4 FYI do not feel pressured to read the code in detail at this point, but ideas for whether or not you are happy to restructure the collisions input struct would be very useful).

Plots of the simulated (fast) ion distribution function: fokker-planck-relaxation-slowing-down-init_pdf_vs_vperp_vpa_iz01_ir01_ion fokker-planck-relaxation-slowing-down_pdf_vs_vperp_vpa_iz01_ir01_ion

LucasMontoya4 commented 2 months ago

@mrhardman I can't really tell (from what I know so far) how much editing elsewhere in the code it would take to change the collisions_input struct, but it sounds like it would be a good idea in future no? Maybe we could have the collisions_input struct contain structs for each collision operator (krook::krook_struct, FP::Fokker_Planck_struct etc.)?

mrhardman commented 2 months ago

@mrhardman I can't really tell (from what I know so far) how much editing elsewhere in the code it would take to change the collisions_input struct, but it sounds like it would be a good idea in future no? Maybe we could have the collisions_input struct contain structs for each collision operator (krook::krook_struct, FP::Fokker_Planck_struct etc.)?

This is the kind of thing that I would like to do. It would not be too much effort, but the changes might touch several files. These changes are not directly related to my cross-species collision operator, so they could go in a separate PR focussing on changing the existing input options. @johnomotani do you agree with the idea, should I make separate PR for the refactoring the existing inputs?

johnomotani commented 2 months ago

The input restructuring sounds like a good idea, and I'm very in favour of doing that kind of preparatory change in a small PR that we can merge early to avoid conflicts later. :+1:

mrhardman commented 2 months ago

The input restructuring sounds like a good idea, and I'm very in favour of doing that kind of preparatory change in a small PR that we can merge early to avoid conflicts later. 👍

Now in progress in #202. You may object to some of the input renaming, but I hope that we can find a way where we are all happy : )

mrhardman commented 2 months ago

FYI, the test moment_kinetics/test/fokker_planck_time_evolution_tests.jl also fails locally on my machine, which apparently can only be due to the change of the frequency_option choice, except for the fact that this doesn't make sense at all, because these tests passed for the #202 branch @johnomotani another mystery?

EDIT: This likely has to do with a poor choice for the default Zi for collisions.

johnomotani commented 2 months ago

Those tests started failing at 0b03e9b, which was before the merges...

Edit: sorry, you've fixed it already!

mrhardman commented 2 months ago

I'm now happy for this to be merged (or to have a longer discussion about the content), as I have all the features that I need for the short term.

johnomotani commented 2 months ago

We can ignore the failing macOS parallel tests here - the cause is some change in the OpenMPI setup, which I've worked around in #206, so those CI tests should be fixed when we merge into master.