Add subgrid-scale (SGS) turbulence model framework

#407 Merged at 47eb089
Repository
enzo-mhdsgs
Branch
week-of-code
Repository
enzo-dev
Branch
week-of-code
Author
  1. Philipp Grete
Reviewers
Description

This PR adds support for SGS turbulence models including

  • several model families (nonlinear, dissipative and scale-similarity)
  • hydro and MHD (Dedner) support
  • framework for explicit filtering of fluid quantities

Comments (15)

    1. Philipp Grete author

      While the roots of both projects are related, this is a complete new implementation from scratch. The main difference is that the model described (and implemented) in Schmidt & Federrath (2011) is a pure hydro model based on grid-scale quantities with a dynamical equation for the kinetic SGS energy, this implementation features hydro and MHD with the possibility of explicit filtering (i.e., resolved quantities on scales larger than the grid scales) and instantaneous kinetic and magnetic SGS energies, i.e., there's no evolution equations for the SGS energy (and therefore no intermediate reservoir that could be used for feedback purposes) but SGS energies are calculated "ad-hoc". Having said that, if anyone is interested in a dynamical MHD SGS framework (e.g., for use with feedback mechanisms in simulations) I'd be happy to get in touch.

  1. Brian O'Shea

    Would you mind doing the following?

    1. updating run/MHD/3D/StochasticForcing/StochasticForcing.enzo so that it only goes for 3 dynamical times (and thus only runs for a minute or so on a modern-ish laptop)
    2. modifying run/MHD/3D/StochasticForcing/StochasticForcing.enzotest so that pushsuite = True ?

    Thank you!

    1. Philipp Grete author

      Done (also updated the docs to the new format). The test run now takes ~70 sec on a single i5-7200U core.

  2. Philipp Grete author

    What are other people's opinions on Nathan's comment? What's the preferred way of taking care of the typedefs I introduce? I personally have no strong preference.

    1. John Wise

      I would agree with Nathan's comment because the variable names are so generic. I would preface them with SGS or the like.

  3. John Wise

    @bwoshea and @ngoldbaum : Do you think this PR is ready for approval after passing the test suite? Any more review comments for @pgrete ?

    1. Brian O'Shea

      Sorry for the delay in reply, @jwise77 . I ran the test suite, and this passes all pre-existing tests (with the new SGS test failing because there’s no pre-existing answers from the gold standard, which is to be expected). I’ll make one more pass through the code, but I think it’s ready to go.

  4. Philipp Grete author

    I updated the PR in order to make the vector/tensor index variable names unique/less general, e.g., XX ->`SGSXX etc.

  5. Brian O'Shea

    @pgrete , I’ve made some small notes below but overall this looks quite good to me.

    1. Brian O'Shea

      @pgrete , I looked over the code again and it looks good to go! @jwise77 and @ngoldbaum , what do you think?

  6. Philipp Grete author

    I just updated the PR to include the latest comments:

    • Updated SGS function names
    • Fixed indentation and extra whitespace in SGS SourceTerms
    • Fixed inded typo SGS JacB typo in Grid destructor.
    • Changed printing debug messages for SGS to verbose debug1 mode

    The last open item (for discussion) is probably whether the memory allocation should happen where it is right now or in the grid constructor, see my comment below.

    1. Brian O'Shea

      I found your comment compelling; that is to say, no allocations take place in the Grid constructor by many other routines, so it would actually be going against current practice in Enzo to allocate your memory in the constructor. If you did so, it’d end up being more confusing in the long run. So, I would say leave it as it is, because the alternative is fixing an awful lot of other code, which is outside the scope of this PR.

  7. Philipp Grete author

    I just pushed the hopefully final update in which I addressed the merge conflict due to recent merged PRs and made the SGS array handling in the Grid constructor/destructor conditional as per Britton’s suggestion.