Bug fix of density floor in PPM solver

#333 Merged at d784c7f
  1. Andrew Emerick

Previously density floor as implemented in euler.F was hard coded and applied only to the density field, rather than both density and inverse density. This change consistently applies the density floor to both and does so using the previously existing SmallRho parameter. No new parameters are added. SmallRho default is 1E-30, which is substantially different from the previous hard coded density floor of 1E-3 (both in code units).

Comments (8)

  1. John Wise

    This all looks good to me. I think when we get consensus (which I think we're there or very close) on the list, I can go ahead and approve this change.

  2. Brian OShea

    This looks good to me, and passes the test suite. You've changed the behavior of the parameter SmallRho, however. Please update the documentation (i.e., the description of "SmallRho" in doc/manual/source/parameters/hydro.rst) to reflect that this no longer just applies to EvolveLevel_rk2!

  3. Greg Bryan

    This looks good to me as well. I agree, with the addition to the docs, I think we're set to go.

  4. Andrew Emerick author

    Just updated the docs. Moved the description of the parameter under the general hydro parameters heading and added mention of the new functionality.

  5. Andrew Emerick author

    Hi all. I wasn't aware that bitbucket automatically adds new commits to any outstanding pull requests. I was trying to submit a new pull request for a separate feature and noticed this happened. I was able to back out the change.

    Brian, if you're O.K. with the doc updates can you approve the request so I can submit my next one (unless there is a better way to go about doing what I tried to do... I'm relatively new to this)

    1. Brian OShea

      One small suggestion for an improvement to the docs: Instead of "Enforced when using the PPM solver in euler.F " maybe it's worth being more specific - "Enforced when using the PPM solver in euler.F (HydroMethod = 0)". Other than that, I think this is ready to get merged.

  6. Brian OShea

    This PR passes the test suite at changeset at d784c7f . Andrew has made the change I requested to the docs, so I'm going to approve and merge.