Improve the handling of unphysical results in Hydro_HancockPredict() by hsinhaoHHuang · Pull Request #502 · gamer-project/gamer

hsinhaoHHuang · 2026-01-03T05:32:19Z

Motivations

Some numerical instabilities were seen in isolated galaxy simulations with feedback and in hydrodynamical cosmological simulations. The unphysical results in the half-step prediction in the MHM scheme may be the source leading to those artificial explosions.
The original negative pressure check inside Hydro_HancockPredict() was performed on fcPri[f][4], which is the input value before the update. The negative pressure after the update may not be detected, and a floored value is returned at the end.

Goal

To improve the accuracy and stability of the half-step Hancock prediction in the MHM scheme.

Changes

Bring back the check of negative pressure after the half-step update.
Add a velocity check against dh/dt in the unphysical checks.
Add a three-stage rescue iteration locally when the unphysical results are found.
- First, recalculate the half-step update with more substeps and with the midpoint integration method.
  - Ideally, it should help and could maintain the desired slope. In practice, however, this is not helpful to rescue the unphysical results most of the time. Therefore, in the default setup, MHM_REPREDICT_ITER_NUM = 2, this will NOT be used.
- Second, reduce the slope for data reconstruction and then recalculate the half-step update
  - Although it is more diffuse, this usually helps and is not as extreme as the zero-slope method in the third stage.
- Third, reset to the cell-centered variables (i.e., zero slope for the half-step update).
  - This is the original method to deal with the unphysical results, and it is the safest.

Verification Tests

Hydro/Riemann with --flu_scheme=MHM
- Riemann_Prob = [0, 1, 2, 3, 4, 5, 9] was run by ALWAYS triggering the rescuing methods (separately) regardless of whether the original update is unphysical to test the correctness of the repredict solver. With the default parameters for the rescue methods, the results can still match the analytical solutions, and there is not much difference from the original update.
  - The results by running with more substeps look almost identical to the original update; while running by the reduced slope, small differences can be seen at some discontinuities.
- Only Riemann_Prob = 9 can produce negative pressure in the default half-step update. The 3-stage rescue methods are triggered for that, and the final results can match the analytical solution as usual.
  - Running by more substeps actually looks slightly worse, while running by reduced slope looks better than the default.
Isolated galaxy with SN feedback
The previous explosions are not found anymore with the new method.
Cosmological hydrodynamical simulations
The previous explosions are gone after the negative pressure is properly checked and a lower slope is applied.

Future Work

Support SRHD
Turn the hard-coded macros MHM_REPREDICT_* into runtime options

Notes

The default parameters of safety factors and the number of substeps are just rule-of-thumb estimations. They are not theoretically based or empirically optimized.
The rescue methods are kind of experimental. Any suggestions on improving the accuracy, stability, and performance (especially for GPUs) for this PR will be greatly appreciated.

hyschive

@hsinhaoHHuang This PR looks good. Thanks for the great effort!

One concern is that I’m not sure whether this prediction works properly for MHD. Check the related comments in Hydro_HancockPredict_RescueByLowerSlopes() and Hydro_HancockPredict_RescueByHigherSteps().

hyschive · 2026-04-11T07:07:20Z

+   if ( MHM_REPREDICT_ITER_NUM < 1 )
+      Aux_Error( ERROR_INFO, "MHM_REPREDICT_ITER_NUM should be >= 1 for MHM_CHECK_PREDICT !!\n" );
+
+   if ( MHM_REPREDICT_STEPS_SAFE_FAC <= 0 )


Suggested change

if ( MHM_REPREDICT_STEPS_SAFE_FAC <= 0 )

if ( MHM_REPREDICT_STEPS_SAFE_FAC <= (real)0.0 )

hyschive · 2026-04-11T07:11:16Z

+   {
+//    estimate the required numer of sub-steps according to the maximum depletion fraction
+      const real safety_factor    = MHM_REPREDICT_STEPS_SAFE_FAC/(1<<Iter); // e.g., 0.4 -> 0.2 -> 0.1
+      const int  num_substeps_est = (int)ceilf( DeplFracMax/safety_factor );


How about defining CEIL in Macro.h, similar to FLOOR?

hyschive · 2026-04-11T07:12:58Z

+   if ( MHM_REPREDICT_SUBSTEPS_MAX < 1 )
+      Aux_Error( ERROR_INFO, "MHM_REPREDICT_SUBSTEPS_MAX should be >= 1 for MHM_CHECK_PREDICT !!\n" );
+
+   if ( MHM_REPREDICT_SLOPE_SAFE_FAC <= 0  ||  MHM_REPREDICT_SLOPE_SAFE_FAC >= 1 )


Suggested change

if ( MHM_REPREDICT_SLOPE_SAFE_FAC <= 0 || MHM_REPREDICT_SLOPE_SAFE_FAC >= 1 )

if ( MHM_REPREDICT_SLOPE_SAFE_FAC <= (real)0.0 || MHM_REPREDICT_SLOPE_SAFE_FAC >= (real)1.0 )

hyschive · 2026-04-11T07:15:37Z

+   if ( MHM_REPREDICT_ITER_NUM < 1 )
+      Aux_Error( ERROR_INFO, "MHM_REPREDICT_ITER_NUM should be >= 1 for MHM_CHECK_PREDICT !!\n" );


Always print the incorrect value to facilitate debugging. Update other error messages too.

Suggested change

if ( MHM_REPREDICT_ITER_NUM < 1 )

Aux_Error( ERROR_INFO, "MHM_REPREDICT_ITER_NUM should be >= 1 for MHM_CHECK_PREDICT !!\n" );

if ( MHM_REPREDICT_ITER_NUM < 1 )

Aux_Error( ERROR_INFO, "MHM_REPREDICT_ITER_NUM (%d) should be >= 1 for MHM_CHECK_PREDICT !!\n", MHM_REPREDICT_ITER_NUM );

hyschive · 2026-04-11T07:16:53Z

+GPU_DEVICE
+static bool Hydro_HancockPredict_CheckUnphysical( const real fcCon[][NCOMP_LR], const real dt_dh,
+                                                  const EoS_t *EoS, const long PassiveFloor );
+# ifndef SRHD


Suggested change

# ifndef SRHD

#ifndef SRHD

hyschive · 2026-04-12T07:38:01Z

+//                ReducedSlopeRatio : Reduced slope / original slope
+//-------------------------------------------------------------------------------------------------------
+GPU_DEVICE
+void Hydro_HancockPredict_RescueByLowerSlopes( real fcCon[][NCOMP_LR],


IIUC, this routine does NOT change the B field. In other words, it always uses the B field stored in fcCon_init, which is computed using the original data reconstruction scheme. I'm not sure whether this minor inconsistency (i.e., adopting different reconstruction methods for the fluid and B fields) would affect the results.

At a minimum, add a comment about this concern.

This is related to the general comment I made in this review.

hyschive · 2026-04-12T07:55:51Z

+//
+// Parameter   :  fcCon             : Face-centered conserved variables to be updated
+//                fcCon_init        : Input face-centered conserved variables before update
+//                dt_dh2            : 0.5 * Time interval to advance solution / Cell size


Suggested change

// dt_dh2 : 0.5 * Time interval to advance solution / Cell size

// dt_dh2 : 0.5 * time interval to advance solution / cell size

hyschive · 2026-04-12T07:56:36Z

+//                                    negative density and pressure
+//                                    --> It is just the input array Flu_Array_In[]
+//                cc_idx            : Index for accessing g_cc_array[]
+//                MinPres           : Density, pressure, and internal energy floors


Suggested change

// MinPres : Density, pressure, and internal energy floors

// MinPres : Pressure floor

hyschive · 2026-04-12T08:02:47Z

+// reconstruct the face-centered variables with the reduced slope
+   for (int f=0; f<6; f++)
+   for (int v=0; v<NCOMP_TOTAL; v++)
+      fcCon[f][v] = g_cc_array[v][cc_idx] + ReducedSlopeRatio * ( fcCon_init[f][v] - g_cc_array[v][cc_idx] );


Add a comment somewhere in this routine clarifying that this rescue approach assumes a piecewise linear distribution even when adopting the PPM data reconstruction.

hyschive · 2026-04-12T08:05:58Z

+#  define MHM_REPREDICT_ITER_NUM                2
+#  define MHM_REPREDICT_STEPS_SAFE_FAC  (real)0.4
+#  define MHM_REPREDICT_SLOPE_SAFE_FAC  (real)0.9
+#  define MHM_REPREDICT_SUBSTEPS_MAX            4


Clarify the usage and typical ranges of these parameters.

feat: add more checks and rescue methods for Hydro_HancockPredict()

971baf1

hsinhaoHHuang mentioned this pull request Feb 7, 2026

Possible ways to improve the half-step prediction of the MHM_RP scheme #510

Open

13 tasks

hyschive self-requested a review February 11, 2026 03:13

hyschive self-assigned this Feb 11, 2026

hyschive added enhancement hydro Hydrodynamics labels Feb 11, 2026

hyschive reviewed Apr 12, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve the handling of unphysical results in Hydro_HancockPredict()#502

Improve the handling of unphysical results in Hydro_HancockPredict()#502
hsinhaoHHuang wants to merge 1 commit intogamer-project:mainfrom
hsinhaoHHuang:ImproveHydroHancockPredict

hsinhaoHHuang commented Jan 3, 2026

Uh oh!

hyschive left a comment

Uh oh!

hyschive Apr 11, 2026

Uh oh!

hyschive Apr 11, 2026

Uh oh!

hyschive Apr 11, 2026

Uh oh!

hyschive Apr 11, 2026

Uh oh!

hyschive Apr 11, 2026

Uh oh!

hyschive Apr 12, 2026

Uh oh!

hyschive Apr 12, 2026

Uh oh!

hyschive Apr 12, 2026

Uh oh!

hyschive Apr 12, 2026

Uh oh!

hyschive Apr 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	if ( MHM_REPREDICT_STEPS_SAFE_FAC <= 0 )
	if ( MHM_REPREDICT_STEPS_SAFE_FAC <= (real)0.0 )

	if ( MHM_REPREDICT_SLOPE_SAFE_FAC <= 0 \|\| MHM_REPREDICT_SLOPE_SAFE_FAC >= 1 )
	if ( MHM_REPREDICT_SLOPE_SAFE_FAC <= (real)0.0 \|\| MHM_REPREDICT_SLOPE_SAFE_FAC >= (real)1.0 )

		if ( MHM_REPREDICT_ITER_NUM < 1 )
		Aux_Error( ERROR_INFO, "MHM_REPREDICT_ITER_NUM should be >= 1 for MHM_CHECK_PREDICT !!\n" );

	// dt_dh2 : 0.5 * Time interval to advance solution / Cell size
	// dt_dh2 : 0.5 * time interval to advance solution / cell size

	// MinPres : Density, pressure, and internal energy floors
	// MinPres : Pressure floor

Conversation

hsinhaoHHuang commented Jan 3, 2026

Motivations

Goal

Changes

Verification Tests

Future Work

Notes

Uh oh!

hyschive left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants