Refactor composition and species inputs

[mrhardman]

PR #225 should be merged to master for this PR to enter master.

This PR makes a namelist structure for the input TOML for composition and species inputs, allowing inputs for an arbitrary number of species to be given. This has the added bonus of cleaning up part of moment_kinetics_input.jl and reducing the number of duplicated functions for the generating composition struct data in test scripts.

As a part of this refactoring it was convenient (for dealing with file i/o, see PR #233) to remove the @enum feature for the electron_physics input. I am quite satisfied with the electron_physics option functionality in this PR, because we keep the @enum variables, whilst allowing a sensible string input to the TOML. Happy to discuss the pros and cons of this change.

This PR is a stepping stone towards making a fully multi-species version of moment_kinetics, enabling different charge numbers and masses. Further input option refactoring could be achieved in this PR, if desired (@johnomotani is this interesting?).

[johnomotani]

The main change of this PR - better structs for the species/composition variables and separate input file sections for the options - is one we've been wanting for a while. That all looks good.

On the Enums:

• I'd much rather use a standard feature of the language (the Enum), than a similar but non-standard bodge.
• The way we input Enums includes a check the the value in the input file is an allowed value. With the bodge, this is not currently done for electron_physics, and would need to be written by hand each time as a generic function cannot know the allowed values - with the Enum the generic function can get the allowed values from the type.
• It's easier to find the allowed values with Enum - they're defined on a single line, and you can also find them in the REPL

  julia> moment_kinetics.input_structs.electron_physics
  moment_kinetics.input_structs.electron_physics_type
  Enum moment_kinetics.input_structs.electron_physics_type:
  boltzmann_electron_response = 0
  boltzmann_electron_response_with_simple_sheath = 1
  braginskii_fluid = 2
  kinetic_electrons = 3
  kinetic_electrons_with_temperature_equation = 4
  

It's true that the support was incomplete. We have to add it ourselves since TOML doesn't support Enum (since that couldn't be done generically). #236 should complete our Enum support for both input from and output to TOML.

I can see why you wrote test_scripts/dict_merge.jl and test_scripts/species_input_test.jl, but are they actually useful to have in the repo? Unlike the collision operator scripts, I'm not seeing how they're useful for future testing/debugging.

[johnomotani]

If everything in the species struct is now contained in composition, can't we just get rid of species completely? That'd be better for a follow-up PR though, as it'd be nice to merge the electrons branch first.

[mrhardman]

Yes, we could remove species completely. I didn't do so because I wanted to make the minimal number of changes to make it easier for you to approve. Happy to wait for the merge with kinetic electrons (or do it after the merge, if it's me that does the merge here).

[mrhardman]

The main change of this PR - better structs for the species/composition variables and separate input file sections for the options - is one we've been wanting for a while. That all looks good.

Thanks!

On the Enums:

• I'd much rather use a standard feature of the language (the Enum), than a similar but non-standard bodge.
• The way we input Enums includes a check the the value in the input file is an allowed value. With the bodge, this is not currently done for electron_physics, and would need to be written by hand each time as a generic function cannot know the allowed values - with the Enum the generic function can get the allowed values from the type.
• It's easier to find the allowed values with Enum - they're defined on a single line, and you can also find them in the REPL

  julia> moment_kinetics.input_structs.electron_physics
  moment_kinetics.input_structs.electron_physics_type
  Enum moment_kinetics.input_structs.electron_physics_type:
  boltzmann_electron_response = 0
  boltzmann_electron_response_with_simple_sheath = 1
  braginskii_fluid = 2
  kinetic_electrons = 3
  kinetic_electrons_with_temperature_equation = 4
  

But should I put integer or string options in the TOML input to electron_physics? Are both integer and string options possible? If you are sure that enum are the best way forward, can we make a note of what to do in the appropriate set_defaults_and_check_section!() call?

It's true that the support was incomplete. We have to add it ourselves since TOML doesn't support Enum (since that couldn't be done generically). #236 should complete our Enum support for both input from and output to TOML.

OK. I am still not fully happy that we needed to make a custom hack to convert enum to string TOML, but if you are confident that it is robust then I cannot argue : ).

I can see why you wrote test_scripts/dict_merge.jl and test_scripts/species_input_test.jl, but are they actually useful to have in the repo? Unlike the collision operator scripts, I'm not seeing how they're useful for future testing/debugging.

These scripts can indeed be deleted now that the functionality is ported into the main code. I wanted them to convince myself that the refactoring was possible in the way that I envisaged it.

[mrhardman]

I am confused as to why the parallel tests are failing here. The --long tests also fail locally, but I haven't changed anything other than input parameter organisation. Before merging with the kinetic electrons updates from master, the tests were passing. The debug tests indicate that there are some shared-memory issues (or at least unnecessary calls to _block_synchronize), but I haven't changed any of the core code in this PR.

[johnomotani]

I could reproduce the errors by running the 'long tests' (which are included in the parallel tests job on ubuntu) and debug tests on my laptop. I think I've fixed them now.