1. The Enzo Project
  2. Untitled project
  3. enzo-dev
  4. Pull requests

Pull requests

#189 Merged at b631baf
Repository
chummels
Branch
week-of-code
Repository
enzo
Branch
week-of-code

Updating Shear Refinement Criterion

Author
  1. chummels
Reviewers
Description

This updates the shear refinement criterion to a more general method. The old method refined on both vorticity as well as shear, and it made the assumption that you were using PPM, that c_s = 1, and it didn't work in cosmological simulations. For the sake of reproducibility of old results, I have created a new parameter, OldShearMethod, that when set to 1 will use the previous method; however, by default it is set to 0.

It is now generalized to operate with Zeus as well as PPM, to calculate c_s when EOSType == 0, and to only refine on shear (and not on vorticity).

This criterion refines by the following method (taken from the comments in the code):

Absolute Shear: [(dvx/dy + dvy/dx)^2 + (dvz/dy + dvy/dz)^2 + (dvx/dz + dvz/dx)^2 ]^(0.5) where: dvx/dy = [vx(j-1) - vx(j+1)]/[2dy], in units of s^(-1)

In order to use this as a refinement criterion, we need to compare it against the local cell width (ie dx). So we refine when the shearing rate is larger than a user-defined threshold divided by the sound-crossing time of the cell:

     Shear > epsilon * c_s / dx

We can save some extra calculations by removing dx from the denominators of both sides of this equation:

((dvx + dvy)^2 + (dvz + dvy)^2 + (dvx + dvz)^2) / c_sound^2 > epsilon^2

I have updated the docs to reflect this change. I'm also including this modification in a PR to the method paper concurrently.

I also mistakenly included a new machine file for SDSC Gordon in this PR.

UPDATE1: Addressed all of Sam Skillman 's comments. Re-indented everything with spaces instead of tabs. Broke the main loop into two separate loops. Removed commented potentially-confusing lines from Gordon Makefile. Tested and gives same results as old version when "OldShearMethod = 1" is set (not default).

UPDATE2: Addressed the suggestion by Greg Bryan to remove the GetUnits call from the code.

Comments (5)

  1. Sam Skillman

    Hi Cameron,

    This looks good to me. Just a few comments:

    Should we hg rm the gordon machine file, or do you think it is a stable/recommended build file that should be included in the repo?

    Would you mind going through and fixing some of the indentation? This has been a pet peeve of mine in the past dealing with some of the darker corners of enzo and it makes it a lot easier on the eyes if we attempt to get rid of the use of tabs in as many cases as possible. In particular, you can see how there are some inconsistencies below in e.g. line 148 of G_FCTBRBS.C and throughout the majority of the rest of that file after that.

    1. chummels author

      The Gordon Machine file is good. I've used it, and it works well. It makes sense to leave it in, I think.

      I can fix the indentation if need be. I left it the way it was primarily since I was only tweaking the way it had been done before, and with all of the nested loops, things were getting super-indented, which was also problematic to read. But I can modify that.

      1. Sam Skillman

        Okay, sounds good -- I left a note below on the optimization flags for the machine file.

        For the indentation I'm looking the diff below where new line 155 is different than old line 85 in terms of indentation. I am pretty sure this is due to mixing of tabs/spaces but I'm not completely sure.

  2. Greg Bryan

    This looks good to me -- the new definition seems reasonable, and it's good to retain backward compatibility. The one thing I see in reviewing the code is that the call to GetUnits is unnecessary -- the returned units are never used. I think it would improve readability if that were removed. Otherwise, good to go!

    1. chummels author

      Oh yeah, that was a holdover for when I was using Temperature to calculate the sound speed instead of pressure. I'll remove that.