Skip to content
Snippets Groups Projects

Add ANARCHY-DU and refactor `hydro_properties.c`

Merged Josh Borrow requested to merge add-anarchy-du into master

This MR does two things:

  • Adds ANARCHY-DU, a variant of the ANARCHY scheme with higher (default) thermal diffusion but based on Density-Energy SPH
  • Re-factors hydro_properties.c. The ANARCHY-DU scheme requires different defaults than the ANARCHY-PU and PressureEnergyMorris schemes so I thought it was time to break these defaults out into a file on their own. Each directory in src/hydro now has it's own src/hydro/*/hydro_defaults.h which contains:
    • Default parameters for AV, and diffusion,
    • The const_viscosity_beta value is defined here as it differs for the Planetary scheme @jkeger
    • A new constant, hydro_props_default_viscosity_alpha_feedback_reset is made available such that particles can have their AV value reset after a feedback event. For constant AV schemes, this is the same as the default artificial viscosity parameter, but for variable schemes this is set to the maximal AV.

This second change allows a number of #ifdef's throughout the code to be removed and provides a solution to #576 (closed).

I also had to re-factor the Default scheme to get it to compile with these changes as it used a random constant in const.h that I removed. Now in that scheme the diffusion alpha is a particle-carried property read from SPH:diffusion_alpha. Also, the documentation was updated for a number of schemes that was still the same as their parent.

I've tested this on the 1D sod for all schemes, and they all compile and pass this (apart from Default but that never works anyway...)

I know that this MR touches way too many files, but that was unavoidable given that I was refactoring every scheme.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Josh Borrow added 1 commit

    added 1 commit

    Compare with previous version

  • Josh Borrow added 1 commit

    added 1 commit

    • 8d5f7091 - Added new hydro_defaults.h files to src/Makefile.am

    Compare with previous version

  • Author Developer

    Oh crap, I accidenatlly changed a bunch of the examples files. Fixing that now.

  • Josh Borrow added 2 commits

    added 2 commits

    • 84a56f9a - remedied incorrect changes to yaml files
    • e1e04042 - Remedied incorrect changes to shell files

    Compare with previous version

  • Author Developer

    Sorry :stuck_out_tongue:

    Most of the lines should just be copies of the hydro_defaults.h file for the (now many) schemes...

    This one is pre-Jenkins approved apparently!

  • Yes, looks daunting but should not be too problematic.

    Can we document the SPH section of the parameter file in the RTD while this happens?

  • Author Developer

    That would probably be wise...

  • Author Developer

    I'll update the documentation yeah, I'll include the new defaults, etc.

  • Also, can the constants not used in a given scheme be removed from the headers? For instance the gizmo header does not need the viscosity terms. Or the gadget header does not need the diffusion and variable visosity constants. Makes the whole thing easier to maintain as we don't have unused constants to worry about.

  • Author Developer

    I don't think they can, as they are used in the hydro_properties.c for reading from the files. What we could do is define different reading routines from the parameter file, but that comes along with it's own problems.

  • Author Developer

    Actually, writing a function for each might be nice as it would leave 'unused' parameters in unused_parameters.yml for the individual runs. Hmm.

  • Author Developer

    I suppose those changes would be:

    For each scheme, rename the hydro_defaults.h file to hydro_parameters.h(?)

    Add two structs to that file

    struct viscosity_global_data {
      float alpha;
      ...
    }
    
    struct diffusion_global_data {
      float alpha;
      ...
    }

    Add a function for each scheme to hydro.h (it may make more sense to keep this in hydro_parameters.h, but that breaks with e.g. the chemistry convention)

    static INLINE void hydro_init_backend(struct swift_params* parameter_file,
                                          const struct unit_system* us,
                                          const struct phys_const* phys_const,
                                          struct viscosity_global_data* viscosity,
                                          struct diffusion_global_data* diffusion) {
      ...
      /* Do the reading from the parameter file here */
      ...

    Include that struct in hydro_properties.h and stick them where the current viscosity and diffusion structs are.

    Add to hydro_properties.c a call to hydro_init_backend.

    Thoughts?

    Edited by Josh Borrow
  • Can we put the viscosity and diffusion structures inside the hydro_props. For schemes where there is nothing needed, they just end up as empty (zero-byte) structures. That way the common stuff only needs defining once.

  • Author Developer

    Yes, sorry -- that's what I meant. Those structures would live inside the hydro_properties structure where the current viscosity and diffusion strcuts (that are fixed) live, to now make them implementation dependent.

  • Note that the chemistry convention is not great and I want to improve it. The feedback may be a better design. We could add a viscosity_init() and diffusion_init() to the hydro_parameters.h and call these from the "master" hydro_properties_init() function.

  • Author Developer

    Just to clarify, you mean:

    • hydro_parameters.h contains:
      • viscosity_global_data struct, which is included inside the global hydro_properties
      • diffusion_global_data struct, which is included inside the global hydro_properties
      • viscosity_init that sets the parameters in the viscosity_global_data struct
      • diffusion_init that sets the parameters in the diffusion_global_data struct
      • Global defaults and other #ifdefs
    • hydro.h contains:
      • hydro_init_backend that calls viscosity_init and diffusion_init, which is then itself called inside hydro_properties.c
  • hydro_properties.c contains hydro_init_backend that calls viscosity_init and diffusion_init.

  • Do we want these updates in this branch or shall I merge this already?

    This is mostly refactoring so I guess we could do these last changes in the same branch.

  • Author Developer

    Yeah I’d rather do the final changes in this branch otherwise we’re gonna cause two huge diffs right next to each other in the history

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
Please register or sign in to reply
Loading