Krook 'collisions'

[johnomotani]

Add a Krook collision operator. Needed this to make 1D1V simulations look nicer for EFTC!

Also includes a script to print some dimensional parameters from a simulation (e.g. ion-ion collision frequency).

TODO:

• [x] test - at least a regression test, including the moment-kinetic versions, is needed

[mrhardman]

I have separately developed Krook collision operator code for 1V and 2V, and extended the manufactured solutions tests to accommodate this feature. The relevant commits are

https://github.com/mabarnes/moment_kinetics/commit/6bd406efd07d677fe2ef175447ccbef9cd05b189 https://github.com/mabarnes/moment_kinetics/commit/d5c366fafec352f64f4e84e328808c77dc0246a5 https://github.com/mabarnes/moment_kinetics/commit/97c788e607320c852c4dd5fb9c40b83db43166c4 https://github.com/mabarnes/moment_kinetics/commit/a6f06ccbbb37de8be113cb31d9c2f28c7fbdf52c

[johnomotani]

@mrhardman I'm proposing to merge this PR, as this Krook collision operator supports moment-kinetic modes, and has a density/temperature dependent collision frequency. Are you OK with that? It will need some merging with your MMS tests when you make a PR for those.

[mrhardman]

@mrhardman I'm proposing to merge this PR, as this Krook collision operator supports moment-kinetic modes, and has a density/temperature dependent collision frequency. Are you OK with that? It will need some merging with your MMS tests when you make a PR for those

I would prefer the MMS test options to be included now, since it seems that you only have a regression test here. The MMS tests are quite orthogonal to the rest of my collision operator development in that branch. Can we discuss offline what the barriers to including the MMS tests are?

[johnomotani]

@mrhardman I don't see any barrier to adding the MMS test. Please feel free to push it onto this branch. I wanted to add the regression test as something that would be run in the CI, as we don't have that set up for any of the MMS tests yet, and they may be too computationally heavy to run in CI anyway.

[mrhardman]

I am starting to merge in the appropriate features. The job may take some time as even the manufactured solutions module appears to have diverged from the initial state where https://github.com/mabarnes/moment_kinetics/commit/d5c366fafec352f64f4e84e328808c77dc0246a5 was committed to. If this merge is urgent, we should discuss.

[mrhardman]

What's the downside of having reference parameters specified?

So long as it is clear that all evolved quantities are normalised, I will have to be happy. In HPC codes that I have worked with previously all small numbers are eliminated from the equations in favour of normalised parameters which enter the input file, and are computed externally to the HPC code. This also gave the option of making physics studies where unphysical parameter scans can be supported (changing the mass of particles, but not collision frequency, for example). In an ideal implementation we would use all reference parameters in a separate module and not propagate their use into the main code, where only normalised quantities are used. This might be what you are already doing -- in which case I cannot find fault.

[johnomotani]

So long as it is clear that all evolved quantities are normalised, I will have to be happy. In HPC codes that I have worked with previously all small numbers are eliminated from the equations in favour of normalised parameters which enter the input file, and are computed externally to the HPC code. This also gave the option of making physics studies where unphysical parameter scans can be supported (changing the mass of particles, but not collision frequency, for example). In an ideal implementation we would use all reference parameters in a separate module and not propagate their use into the main code, where only normalised quantities are used. This might be what you are already doing -- in which case I cannot find fault.

Agree with all of this. I would say that I think it's nicer if 'unphysical' setups have to be specified explicitly, with physically consistent things being the default. It does seem like a good idea to separate the 'reference parameters' into a separate module so that the separation is more obviously enforced - that they are defined, but only used in one or two places within the setup. I'll make a commit to tidy this up a bit - edit: see 49f41f8.

[mrhardman]

An initial attempt to port the modifications to the manufactured solutions module to permit the MMS testing of the Krook operator is here: https://github.com/mabarnes/moment_kinetics/commit/030fbe780dd93d3d0893b2ed8dae54314297440a

The O(1) errors in the test of u|| and p|| likely arise from conflicting normalisation conventions.

[mrhardman]

Make clearer that T_over_Tref is not the normalised temperature as defined in the code.

Why not avoid using T_over_Tref entirely and put

n * vth^(-3)

in the collision operator factor?

[johnomotani]

Implementing collision operator in terms of vth - I like that idea. I guess however we end up defining cref, we'd choose to have vth(norm) = vth(unnorm)/cref, so the implementation in terms of vth would keep working for any choice.

Diagnostic output for collision frequency - that would be useful, and should be an easy thing to implement.

[johnomotani]

...both upgrades now made.

After adding a function that calculates collision frequency (and a little bit of refactoring of makie_post_processing), adding plots of the collision frequency in makie_post_processing only took 9 lines :sunglasses: https://github.com/mabarnes/moment_kinetics/blob/c7735e160c3082053fc1d2ba3b14ea5e201d6ec3/src/makie_post_processing.jl#L73 https://github.com/mabarnes/moment_kinetics/blob/c7735e160c3082053fc1d2ba3b14ea5e201d6ec3/src/makie_post_processing.jl#L1210-L1213 https://github.com/mabarnes/moment_kinetics/blob/c7735e160c3082053fc1d2ba3b14ea5e201d6ec3/src/makie_post_processing.jl#L1333-L1336

[johnomotani]

PS the spatially-varying collision frequency is O(1), at least in the 1V test