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

Pull requests

#261 Merged at 8333f56
Repository
enzo-2.x-larrue
Branch
week-of-code
Repository
enzo-dev
Branch
week-of-code

Small fixes/improvements found while doing testing

Author
  1. James Larrue
Reviewers
Description

There are three changes in this PR:

1) Use only the required number of velocity fields with Hydro Shock scenarios. See commits 1881953 and 77f46a7 (HydroShockTubesInitialize.C, ShockInABoxInitialize.C, and Grid_HydroShockTubesInitializeGrid.C). This change makes the InteractingShockWaves test give different values, because the center zone is no longer initialized with y- and z-velocities of 1.0 by default when the problem is 1D. Previously, this would cause the grid refinement to occur unnecessarily on some cells, due to the y- and z-velocity slopes near the center zone.

2) Set default initialization values for flux arrays. See commit 4c31c20 (Grid_[xyz]EulerSweep.C). In a scenario with LLF for the Riemann Solver, I was getting strange results because the flux arrays were not initialized. I added a 'default' case to the switch statement that handles the initialization of the flux arrays.

3) Initialize properly the dummy array used to initialize the external boundary face. See commit 1b7d47f (InitializeNew.C). For whatever reason, the dummy array was set to size MAX_DIMENSIONS when it should have been set to size NumberOfBaryonFields.

Comments (13)

    1. James Larrue author

      Actually, that is as expected, and as desired.

      The old code was using 3 velocity directions for InteractingBlastWaves, even though the grid rank is only 1. In addition, the old code assigned non-zero default values in the central region of the y- and z-velocities. (InteractingBlastWaves.enzo does not specify y- and z-velocities, since the test is supposed to be 1D.) The result was an extra cell being refined by slope due to the y-velocity. Since this test is 1D, the y-velocity should not exist and its slope should not be causing cells to be flagged for refinement.

      The new version uses only the x-velocity, since the grid rank is 1. This means the grid refinement will be based only on the x-velocity and not on the non-existent velocities. In addition, the default velocity of the central regions is now zero, though this fact does not make any difference now that the y- and z-velocities are correctly never created.

      The commit that created these changes is:

      https://bitbucket.org/james_larrue/enzo-2.x-larrue/commits/18819530f8317fc5cdb9ddc8f45de45edb7c6844

      (NB: The commit comments also talk about ShockInABox being different, but a later commit made that test give the same results.)

      1. James Larrue author

        I was thinking about how to test this. If you ran the old code with the following parameters added to the InteractingBlastWaves.enzo file:

        HydroShockTubesCenterVelocityY = 0.0
        HydroShockTubesCenterVelocityZ = 0.0
        

        then it should give the same results as this PR.

          1. James Larrue author

            Yes, it is true.

            I created baseline test results for InteractingBlastWaves with the original code (46c7166) and with the above two lines added to the file ./run/Hydro/Hydro-1D/InteractingBlastWaves/InteractingBlastWaves.enzo . I then re-ran the same code against the new InteractingBlastWaves baseline (i.e. the same code that created the baseline was run against the baseline) and received:

            Test Summary Sims Not Finishing: 0 Tests Passed: 30 Tests Failed: 0 Tests Errored: 0

            I then ran this version of the code (1b7d47f) against the new InteractingBlastWaves baseline and received:

            Test Summary Sims Not Finishing: 0 Tests Passed: 16 Tests Failed: 0 Tests Errored: 0

            There are 14 tests that are not performed in the new version of the code, because the y-velocity and z-velocity values do not exist in the new version. (The new version is really 1-D when you say GridRank 1. The old version was always 3-D, even with GridRank 1.) In ${YT_HOME}/src/yt-hg/yt/frontends/enzo/answer_testing_support.py, you can see that the "standard_small_simulation" function will perform 7 tests for each velocity direction: 3 axis (0, 1, 2) * 2 weights (None, Density) + 1 FieldValuesTest. Since we do not have the extra 2 velocity fields in the new version of the code, we have 14 fewer tests (2 velocity fields x 7 tests/field = 14 tests).

            The two versions of the code give the same results for InteractingBlastWaves when the two lines above are added to the original code's configuration file.

    1. James Larrue author

      Actually, that is as expected, and as desired.

      The old code was using 3 velocity directions for InteractingBlastWaves, even though the grid rank is only 1. In addition, the old code assigned non-zero default values in the central region of the y- and z-velocities. (InteractingBlastWaves.enzo does not specify y- and z-velocities, since the test is supposed to be 1D.) The result was an extra cell being refined by slope due to the y-velocity. Since this test is 1D, the y-velocity should not exist and its slope should not be causing cells to be flagged for refinement.

      The new version uses only the x-velocity, since the grid rank is 1. This means the grid refinement will be based only on the x-velocity and not on the non-existent velocities. In addition, the default velocity of the central regions is now zero, though this fact does not make any difference now that the y- and z-velocities are correctly never created.

      The commit that created these changes is:

      https://bitbucket.org/james_larrue/enzo-2.x-larrue/commits/18819530f8317fc5cdb9ddc8f45de45edb7c6844

      (NB: The commit comments also talk about ShockInABox being different, but a later commit made that test give the same results.)

      If you ran the old code with the following parameters added to the InteractingBlastWaves.enzo file:

      HydroShockTubesCenterVelocityY = 0.0
      HydroShockTubesCenterVelocityZ = 0.0
      

      then it gives the same results as this PR.