Refactor namelist inputs for coordinates and other inputs

[mrhardman]

Should we prioritise refactoring the coordinate, evolve_moments, drive, and remaining collisions inputs to use a namelist format?

This may have the benefit of reducing clutter in moment_kinetics_input.jl, as well an making the specification of the defaults unique.

Is there any reason for why we should not refactor the input structure for these inputs?

@johnomotani @LucasMontoya4

[mrhardman]

@johnomotani Comments on #181 (comment).

The structure below is how we might hope to set the coordinate data.

foo_input = OptionsDict("foo" => OptionsDict("ngrid" => 5, "nelement" => 10))
define_coordinate(foo_input, "foo")

However, there are some complications that I will note in case it is helpful. At the moment we have an intermediate input_struct for coordinate data that we fill from .toml data in moment_kinetics_input.jl. Once we fill this mutable struct, we set up the MPI with this information and then we pass that data into another input_struct which is then passed to define_coordinate().

When making other refactors from flat .toml to namelists, I endevoured to use @johnomotani's method of using set_defaults_and_check_section!() to make an OptionsDict, where default values that depend conditionally on other inputs are set appropriately, or in the function immediately following the namelist read (e.g. in

moment_kinetics/moment_kinetics/src/species_input.jl

Line 16 in db370d1

function get_species_input(toml_input)

). Then this dictionary is passed into the pre-defined input struct, e.g., as in

moment_kinetics/moment_kinetics/src/species_input.jl

Line 197 in db370d1

composition = species_composition(; input...)

. This method has the nice benefit of a definitely typed struct as a result of the input data read, and a single location to determine the default values in the struct. It would be good to use this method of setting a struct once and forever throughout the code, if possible, so that reading input data looks somewhat standard for all features. It would be possible to avoid using a struct for output of the input data function (like with the timestepping options, where I think a Dict is passed around, rather than a struct), but then the input data can be changed at run-time elsewhere in the code with potential unexpected consequences.

My comment for reading the coordinate data is therefore that we might want to write the function to read coordinate data to the intermediate (but not mutable) struct grid_input

moment_kinetics/moment_kinetics/src/input_structs.jl

Line 216 in db370d1

struct grid_input

, to permit setting of the MPI communicators within the setup function, before passing the grid_input struct around the code as is done at present.

[johnomotani]

Yeah, I ran into this - got around it by making a function that reads the options and puts them in a NamedTuple. That function can be called inside define_coordinate() or before, and then it's output passed into define_coordinate().

[mrhardman]

So I guess that your function creates a NamedTuple with the MPI communicator by default to be NULL, but otherwise sets up to the MPI using the existing functions? Does that mean that you have to create all the coordinate NameTuples at once?

Is function general enough that you can create coordinates with names that don't already exist in the moment kinetics source? (like here

moment_kinetics/moment_kinetics/test/poisson_radial_polar_tests.jl

Line 97 in aad7e27

polar, polar_spectral = define_coordinate(polar_input, ignore_MPI=true)

).