Add option to compute center of mass to hydro_analysis and lots of smaller changes

Issue #1152 closed
Roland Haas created an issue

The attached set of patches adds the possibility to compute a center of mass location to hydro_analysis. It works by computing the center of mass (via x[idx]*rho[idx]) of the matter in a region of radius Hydro_Analysis_r_core around the point of maximum density. Only points whose density is above a fraction Hydro_Analysis_rho_core_rel_min of the maximum density are considered.

I also include a number of bugfix patches and patches to make Hydro_Analysis more controllable by user input rather than hard-coded values. All patches contain a short description in the patch (ie. the git commit message).

The ones adding extra control and/or warnings are: * Hydro_Analysis: add verbosity_level option to control how much data to output to stdout * Hydro_Analysis: use Fortran modules for prototypes if available * Hydro_Analysis: add parameters to control which interpolator is used * Hydro_Analysis: make parameters steerable * Hydro_Analysis: warn if grid point with maximum value cannot be found on grid * Hydro_Analysis: add parameter Hydro_Analysis_comp_rho_max_every * Hydro_Analysis: add option rho_max_loc_use_rotatingsymmetry180

  • Hydro_Analysis: add option to average the location of multiple identical maxima
  • Hydro_Analysis: fiddle with when to warn about multiple identical maxima

Bugfixes are: * Hydro_Analysis: checkpoint grid scalars * Hydro_Analysis: do reduction in global mode

New feature: * Hydro_Analysis: add routine to compute center of mass in core region

Keyword: Hydro_Analysis

Comments (20)

  1. Frank Löffler
    • removed comment

    0001-Hydro_Analysis-add-option-to-average-the-location-of.patch looks good so far, but I believe it would be nice to be able to say 'allow this only to happen if the identical values are actually within up to 2*dx from each other. That however would require to actually store these locations. Please apply 0001.

  2. Frank Löffler
    • removed comment

    0002-Hydro_Analysis-add-verbosity_level-option-to-control.patch looks good too.

  3. Frank Löffler
    • removed comment

    0003-Hydro_Analysis-use-Fortran-modules-for-prototypes-if.patch: I assume this is preparing following patches, e.g., 0004. Now, these then use the table-interfaces from Fortan. Wouldn't that make this not only optional but required? Do later patches compile without the FORTAN capability?

  4. Roland Haas reporter
    • removed comment

    Replying to [comment:5 knarf]:

    0003-Hydro_Analysis-use-Fortran-modules-for-prototypes-if.patch: I assume this is preparing following patches, e.g., 0004. Now, these then use the table-interfaces from Fortan. Wouldn't that make this not only optional but required? Do later patches compile without the FORTAN capability?

    Fortran does not require prototypes and all functions are all usable without the module. So this patch is purely optional. I like having prototypes if possible since they help me catch errors.

    All patches seem to compile fine without Fortran modules present. I just verified this by removing Fortran from my thornlist, which also disables QuasiLocalMeasures.

  5. Roland Haas reporter
    • removed comment

    Committed 0003, 0004 and 0005 as rev 124, 125 and 126 of Hydro_Analysis (0004 and 0005 modify only warnings are default to the old behaviour). I fully expect this to take a while since some patches are quite extensive. Thank you for taking the time.

  6. Roland Haas reporter
    • removed comment

    Updated 0006 to replace an assert() by a VWarn(1,...) and early return (the situation assert'ed against actually happens regularly in NSNS mergers).

  7. Roland Haas reporter
    • removed comment

    0006, 0007, 0008, 0009, 0010, 0011, 0012, 0013, 0014 all should be applied. They all apply on top of the current trunk. 6, 9 are updated since they contained an update (6) or no longer applied (9). 10 was missing from the first batch, 13 and 14 are new.

  8. Erik Schnetter
    • removed comment

    The centre of mass is not rho x^i / M, but rather sqrt(det(g_ij)) rho x^i / M, where M = sqrt(det(g_ij)) rho (integral signs omitted).

    I don't see how myrho is defined in patch 0006, and it's not currently defined in Hydro_Analysis either. Maybe myrho = rho sqrt(g)?

  9. Roland Haas reporter
    • removed comment

    myrho is declared by DECLARE_MY_VARIABLE(rho) in line 44. I am fine renaming the routine to "centroid" rather than "center of mass". Since this is only used to track the star and move the grids along using just rho is fine. For the actual center of mass one would either have to compute sqrt(det(g_ij)) * w_lorentz * rho or integrate dens. This would be more expensive. It is very easy to change the code so that it takes a parameter specifying what "rho" should be "HYDROBASE::rho" would be the current situation and "GRHYDRO::dens" would be the actual center of mass. Would that be acceptable?

  10. Erik Schnetter
    • removed comment

    It's okay to only implement what you need. If you don't need the true CoM, then there's no need to calculate it. Renaming it to "centroid" (and adding a comment that that is not actually the centre of mass) is fine.

  11. Roland Haas reporter
    • changed status to resolved
    • removed comment

    Applied as git hashes

      0600cfc (0006)
      f68e878 - rename CoM to centroid
      3474885 (0007)
      e6ca32e (0008)
      2b3613b (0009)
      88c0f56 (0010)
      4d7d0e8 (0011)
      9df58fc (0012)
      320dd23 (0013)
      3b4310e (0014)
      448bdad - adds a test case
      e97ae9e - use "-" to separate thorn and variable name
    
  12. Log in to comment