-
Notifications
You must be signed in to change notification settings - Fork 132
MHD Hyperbolic Divergence Cleaning #1086
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a838583
c745398
e98fb01
6798e4b
4d651e8
89a0ac5
ef446f5
2f9b1be
f0b7239
2c29ab2
7db99b4
f3b6a37
32655d0
edc9f5d
64f4d96
96ccf10
f942bb9
4480d18
c642669
bd12ee0
efb124d
3011794
6c61e02
780f9bd
f54854b
4e25edb
178a9bb
f1862ed
3503bb5
059b941
9e04bd7
ba275e8
ba7b4af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| #!/usr/bin/env python3 | ||
| import json | ||
| import math | ||
|
|
||
| # Configuring case dictionary | ||
| print( | ||
| json.dumps( | ||
| { | ||
| # Logistics | ||
| "run_time_info": "T", | ||
| # Computational Domain Parameters | ||
| "x_domain%beg": 0.0, | ||
| "x_domain%end": 1.0, | ||
| "y_domain%beg": 0.0, | ||
| "y_domain%end": 1.0, | ||
| "m": 399, | ||
| "n": 399, | ||
| "p": 0, | ||
| "dt": 0.0004, | ||
| "t_step_start": 0, | ||
| "t_step_stop": 375, | ||
| "t_step_save": 5, | ||
| # Simulation Algorithm Parameters | ||
| "num_patches": 1, | ||
| "model_eqns": 2, | ||
| "alt_soundspeed": "F", | ||
| "num_fluids": 1, | ||
| "mpp_lim": "F", | ||
| "mixture_err": "F", | ||
| "time_stepper": 3, | ||
| "weno_order": 5, | ||
| "mapped_weno": "T", | ||
| "weno_eps": 1.0e-6, | ||
| "null_weights": "F", | ||
| "mp_weno": "F", | ||
| "riemann_solver": 1, | ||
| "wave_speeds": 1, | ||
| "avg_state": 2, | ||
| "bc_x%beg": -1, | ||
| "bc_x%end": -1, | ||
| "bc_y%beg": -1, | ||
| "bc_y%end": -1, | ||
| # Formatted Database Files Structure Parameters | ||
| "format": 1, | ||
| "precision": 2, | ||
| "prim_vars_wrt": "T", | ||
| "rho_wrt": "T", | ||
| "parallel_io": "T", | ||
| # MHD | ||
| "mhd": "T", | ||
| "hyper_cleaning": "T", | ||
| "hyper_cleaning_speed": 2.5, | ||
| "hyper_cleaning_tau": 0.004, | ||
| # Patch 1 - 2D MHD Rotor Problem | ||
| # gamma = 1.4 | ||
| # Ambient medium (r > 0.1): | ||
| # rho = 1 | ||
| # p = 1 | ||
| # v = (0, 0, 0) | ||
| # B = (1, 0, 0) | ||
| # Rotor (r <= 0.1): | ||
| # rho = 10 | ||
| # v has angular velocity of 20 | ||
| "patch_icpp(1)%hcid": 252, | ||
| "patch_icpp(1)%geometry": 3, | ||
| "patch_icpp(1)%x_centroid": 0.5, | ||
| "patch_icpp(1)%y_centroid": 0.5, | ||
| "patch_icpp(1)%length_x": 1.0, | ||
| "patch_icpp(1)%length_y": 1.0, | ||
| "patch_icpp(1)%vel(1)": 0.0, | ||
| "patch_icpp(1)%vel(2)": 0.0, | ||
| "patch_icpp(1)%vel(3)": 0.0, | ||
| "patch_icpp(1)%pres": 1.0, | ||
| "patch_icpp(1)%Bx": 5.0 / math.sqrt(math.pi * 4.0), | ||
| "patch_icpp(1)%By": 0.0, | ||
| "patch_icpp(1)%Bz": 0.0, | ||
| "patch_icpp(1)%alpha_rho(1)": 1.0, | ||
| "patch_icpp(1)%alpha(1)": 1.0, | ||
| # Fluids Physical Parameters | ||
| "fluid_pp(1)%gamma": 1.0e00 / (1.4e00 - 1.0e00), | ||
| "fluid_pp(1)%pi_inf": 0.0, | ||
| } | ||
| ) | ||
| ) |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,9 +1,14 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #:def Hardcoded2DVariables() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! Place any declaration of intermediate variables here | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| real(wp) :: eps | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| real(wp) :: eps, eps_mhd, C_mhd | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| real(wp) :: r, rmax, gam, umax, p0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| real(wp) :: rhoH, rhoL, pRef, pInt, h, lam, wl, amp, intH, intL, alph | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| real(wp) :: factor | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| real(wp) :: r0, alpha, r2 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| real(wp) :: sinA, cosA | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| real(wp) :: r_sq | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! # 207 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| real(wp) :: sigma, gauss1, gauss2 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! # 208 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -183,7 +188,43 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| q_prim_vf(E_idx)%sf(i, j, 0) = 3.e-5_wp | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| end if | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| case (252) ! MHD Smooth Magnetic Vortex | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! case 252 is for the 2D MHD Rotor problem | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| case (252) ! 2D MHD Rotor Problem | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! Ambient conditions are set in the JSON file. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! This case imposes the dense, rotating cylinder. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! gamma = 1.4 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! Ambient medium (r > 0.1): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! rho = 1, p = 1, v = 0, B = (1,0,0) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! Rotor (r <= 0.1): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! rho = 10, p = 1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! v has angular velocity w=20, giving v_tan=2 at r=0.1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! Calculate distance squared from the center | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| r_sq = (x_cc(i) - 0.5_wp)**2 + (y_cc(j) - 0.5_wp)**2 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! inner radius of 0.1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (r_sq <= 0.1**2) then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! -- Inside the rotor -- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! Set density uniformly to 10 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| q_prim_vf(contxb)%sf(i, j, 0) = 10._wp | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! Set vup constant rotation of rate v=2 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! v_x = -omega * (y - y_c) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! v_y = omega * (x - x_c) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| q_prim_vf(momxb)%sf(i, j, 0) = -20._wp*(y_cc(j) - 0.5_wp) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| q_prim_vf(momxb + 1)%sf(i, j, 0) = 20._wp*(x_cc(i) - 0.5_wp) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ! taper width of 0.015 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else if (r_sq <= 0.115**2) then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+207
to
+219
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: In the rotor initial condition, the comparison radii use default-kind real literals ( Severity Level: Major
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (r_sq <= 0.1**2) then | |
| ! -- Inside the rotor -- | |
| ! Set density uniformly to 10 | |
| q_prim_vf(contxb)%sf(i, j, 0) = 10._wp | |
| ! Set vup constant rotation of rate v=2 | |
| ! v_x = -omega * (y - y_c) | |
| ! v_y = omega * (x - x_c) | |
| q_prim_vf(momxb)%sf(i, j, 0) = -20._wp*(y_cc(j) - 0.5_wp) | |
| q_prim_vf(momxb + 1)%sf(i, j, 0) = 20._wp*(x_cc(i) - 0.5_wp) | |
| ! taper width of 0.015 | |
| else if (r_sq <= 0.115**2) then | |
| if (r_sq <= 0.1_wp**2) then | |
| ! -- Inside the rotor -- | |
| ! Set density uniformly to 10 | |
| q_prim_vf(contxb)%sf(i, j, 0) = 10._wp | |
| ! Set vup constant rotation of rate v=2 | |
| ! v_x = -omega * (y - y_c) | |
| ! v_y = omega * (x - x_c) | |
| q_prim_vf(momxb)%sf(i, j, 0) = -20._wp*(y_cc(j) - 0.5_wp) | |
| q_prim_vf(momxb + 1)%sf(i, j, 0) = 20._wp*(x_cc(i) - 0.5_wp) | |
| ! taper width of 0.015 | |
| else if (r_sq <= 0.115_wp**2) then |
Steps of Reproduction ✅
1. Configure a patch with hardcoded initial condition id hcid=252 (2D MHD Rotor) in the
input (Hardcoded2D is defined in src/common/include/2dHardcodedIC.fpp).
2. Run the solver to reach initialization where the Hardcoded2D macro executes; the rotor
branch starts at src/common/include/2dHardcodedIC.fpp lines around 192..227 and computes
r_sq at line 204: "r_sq = (x_cc(i) - 0.5_wp)**2 + (y_cc(j) - 0.5_wp)**2".
3. Initialization evaluates the radius tests at lines 207 ("if (r_sq <= 0.1**2) then") and
219 ("else if (r_sq <= 0.115**2) then") to classify inside/transition/outside cells.
4. Because the literals 0.1**2 and 0.115**2 are default‑kind reals while r_sq and other
variables use kind _wp, cells with sqrt(r_sq) numerically extremely close to 0.1 or 0.115
can be classified differently than when using _wp‑suffixed literals; this reproduces the
misclassification at initialization for boundary grid cells. (This is a marginal numerical
precision issue localized to hcid=252; the code path and lines above show the exact
location.)Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/common/include/2dHardcodedIC.fpp
**Line:** 207:219
**Comment:**
*Type Error: In the rotor initial condition, the comparison radii use default-kind real literals (`0.1**2`, `0.115**2`) while `r_sq` is `real(wp)`, causing mixed‑kind arithmetic and slight precision inconsistencies that can misclassify cells very close to the intended radius thresholds. Use `_wp` suffixed literals to keep everything in the same kind.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Add an else branch to the MHD Rotor problem to initialize the ambient region's density and momentum. [possible issue, importance: 8]
| case (252) ! 2D MHD Rotor Problem | |
| ! Ambient conditions are set in the JSON file. | |
| ! This case imposes the dense, rotating cylinder. | |
| ! | |
| ! gamma = 1.4 | |
| ! Ambient medium (r > 0.1): | |
| ! rho = 1, p = 1, v = 0, B = (1,0,0) | |
| ! Rotor (r <= 0.1): | |
| ! rho = 10, p = 1 | |
| ! v has angular velocity w=20, giving v_tan=2 at r=0.1 | |
| ! Calculate distance squared from the center | |
| r_sq = (x_cc(i) - 0.5_wp)**2 + (y_cc(j) - 0.5_wp)**2 | |
| ! inner radius of 0.1 | |
| if (r_sq <= 0.1**2) then | |
| ! -- Inside the rotor -- | |
| ! Set density uniformly to 10 | |
| q_prim_vf(contxb)%sf(i, j, 0) = 10._wp | |
| ! Set vup constant rotation of rate v=2 | |
| ! v_x = -omega * (y - y_c) | |
| ! v_y = omega * (x - x_c) | |
| q_prim_vf(momxb)%sf(i, j, 0) = -20._wp*(y_cc(j) - 0.5_wp) | |
| q_prim_vf(momxb + 1)%sf(i, j, 0) = 20._wp*(x_cc(i) - 0.5_wp) | |
| ! taper width of 0.015 | |
| else if (r_sq <= 0.115**2) then | |
| ! linearly smooth the function between r = 0.1 and 0.115 | |
| q_prim_vf(contxb)%sf(i, j, 0) = 1._wp + 9._wp*(0.115_wp - sqrt(r_sq))/(0.015_wp) | |
| q_prim_vf(momxb)%sf(i, j, 0) = -(2._wp/sqrt(r_sq))*(y_cc(j) - 0.5_wp)*(0.115_wp - sqrt(r_sq))/(0.015_wp) | |
| q_prim_vf(momxb + 1)%sf(i, j, 0) = (2._wp/sqrt(r_sq))*(x_cc(i) - 0.5_wp)*(0.115_wp - sqrt(r_sq))/(0.015_wp) | |
| end if | |
| case (252) ! 2D MHD Rotor Problem | |
| ... | |
| if (r_sq <= 0.1_wp**2) then | |
| ... | |
| else if (r_sq <= 0.115_wp**2) then | |
| ... | |
| else | |
| ! -- Ambient medium -- | |
| q_prim_vf(contxb)%sf(i, j, 0) = 1._wp | |
| q_prim_vf(momxb)%sf(i, j, 0) = 0._wp | |
| q_prim_vf(momxb + 1)%sf(i, j, 0) = 0._wp | |
| end if |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: In the Gaussian divergence pulse case, sigma is read directly from input and then used as a divisor without validation; if the user forgets to set it or sets it to a non-positive value, this will cause a division by zero or mathematically invalid configuration at runtime. [possible bug]
Severity Level: Major ⚠️
- ❌ Initialization for Gaussian divergence pulse (hcid=260) fails.
- ⚠️ Produces NaN/INF B‑field if sigma is zero or negative.| C_mhd = eps_mhd*sigma*sqrt(pi)*0.5_wp | |
| if (sigma <= 0._wp) then | |
| call s_mpi_abort("Gaussian Divergence Pulse (hcid=260): sigma must be positive.") | |
| end if | |
Steps of Reproduction ✅
1. Configure a patch to use hardcoded initial condition hcid=260 (Gaussian Divergence
Pulse) in the input so the case (260) branch in src/common/include/2dHardcodedIC.fpp is
executed.
2. Provide patch parameters with a(3)=0 or omit setting a(3) so patch_icpp(patch_id)%a(3)
resolves to zero; the code assigns sigma at src/common/include/2dHardcodedIC.fpp line 249:
"sigma = patch_icpp(patch_id)%a(3)".
3. Initialization computes the B‑field using erf((x_cc(i) - 0.5_wp)/sigma) at line 254.
With sigma == 0 the division produces an invalid argument (INF/NaN) or a runtime fault
depending on compiler/FP settings, observable as NaNs in B arrays or an initialization
failure.
4. Observe the failure by running the solver to initialization and inspecting the
initialized q_prim_vf(B_idx) array or checking for abnormal program termination. Adding
the explicit check (sigma <= 0) at line 251 and aborting with a clear message prevents the
invalid initialization and makes the failure mode explicit.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/common/include/2dHardcodedIC.fpp
**Line:** 251:251
**Comment:**
*Possible Bug: In the Gaussian divergence pulse case, `sigma` is read directly from input and then used as a divisor without validation; if the user forgets to set it or sets it to a non-positive value, this will cause a division by zero or mathematically invalid configuration at runtime.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
danieljvickers marked this conversation as resolved.
Show resolved
Hide resolved
danieljvickers marked this conversation as resolved.
Show resolved
Hide resolved
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -363,6 +363,9 @@ contains | |||||
| ! Damage state variable | ||||||
| if (cont_damage) dbvars = dbvars + 1 | ||||||
|
|
||||||
| ! Hyperbolic cleaning for MHD | ||||||
| if (hyper_cleaning) dbvars = dbvars + 1 | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: The hyperbolic cleaning scalar is counted as an output variable whenever Severity Level: Major
|
||||||
| if (hyper_cleaning) dbvars = dbvars + 1 | |
| if (mhd .and. hyper_cleaning) dbvars = dbvars + 1 |
Steps of Reproduction ✅
1. Configure a run that requests Binary formatted post-processing output (format == 2) and
enable hyperbolic cleaning while MHD is disabled. The dbvars counter is computed inside
s_initialize_data_output_module in src/post_process/m_data_output.fpp; the hyper_cleaning
increment is at the lines shown above (lines 366-367).
2. Start the application so that s_initialize_data_output_module runs and sets dbvars (the
code path that computes dbvars includes the two lines at 366-367). Because hyper_cleaning
is true, dbvars is incremented unconditionally at 367 even though mhd is false.
3. The code path that writes the Binary file header in s_open_formatted_database_file
(src/post_process/m_data_output.fpp) writes the declared number of variables into the file
header: write (dbfile) m, n, p, dbvars. This write uses the dbvars value computed above.
4. Later the routines that actually write per-variable binary blocks
(s_write_variable_to_formatted_database_file and related code in the same file) skip
MHD-related variables when mhd is false, so fewer variable blocks are emitted than the
header advertised. The result is a mismatch between header (contains extra hyper_cleaning
variable) and actual data blocks; binary readers or downstream post-processing will
mis-interpret file layout, causing corrupted reads or failures.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/post_process/m_data_output.fpp
**Line:** 367:367
**Comment:**
*Logic Error: The hyperbolic cleaning scalar is counted as an output variable whenever `hyper_cleaning` is true, even if MHD is disabled; this can make the declared number of binary output variables (`dbvars`) larger than the number actually written, corrupting the binary file layout for non‑MHD runs where `hyper_cleaning` might be (mis)enabled.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -117,6 +117,7 @@ module m_global_parameters | |||||
| integer :: b_size !< Number of components in the b tensor | ||||||
| integer :: tensor_size !< Number of components in the nonsymmetric tensor | ||||||
| logical :: cont_damage !< Continuum damage modeling | ||||||
| logical :: hyper_cleaning !< Hyperbolic cleaning for MHD | ||||||
| logical :: igr !< enable IGR | ||||||
| integer :: igr_order !< IGR reconstruction order | ||||||
| logical, parameter :: chemistry = .${chemistry}$. !< Chemistry modeling | ||||||
|
|
@@ -143,6 +144,7 @@ module m_global_parameters | |||||
| integer :: c_idx !< Index of color function | ||||||
| type(int_bounds_info) :: species_idx !< Indexes of first & last concentration eqns. | ||||||
| integer :: damage_idx !< Index of damage state variable (D) for continuum damage model | ||||||
| integer :: psi_idx !< Index of hyperbolic cleaning state variable for MHD | ||||||
| !> @} | ||||||
|
|
||||||
| ! Cell Indices for the (local) interior points (O-m, O-n, 0-p). | ||||||
|
|
@@ -407,6 +409,7 @@ contains | |||||
| b_size = dflt_int | ||||||
| tensor_size = dflt_int | ||||||
| cont_damage = .false. | ||||||
| hyper_cleaning = .false. | ||||||
| igr = .false. | ||||||
|
|
||||||
| bc_x%beg = dflt_int; bc_x%end = dflt_int | ||||||
|
|
@@ -812,6 +815,13 @@ contains | |||||
| damage_idx = dflt_int | ||||||
| end if | ||||||
|
|
||||||
| if (hyper_cleaning) then | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: The hyperbolic cleaning variable is appended to the system state vector whenever Severity Level: Major
|
||||||
| if (hyper_cleaning) then | |
| if (hyper_cleaning .and. mhd .and. n > 0) then |
Steps of Reproduction ✅
1. Configure a run where model_eqns selects the branch that executes the state-vector
annotation code in impure subroutine s_initialize_global_parameters_module (the large
routine that sets up cont_idx, mom_idx, B_idx, etc.). The psi_idx insertion block is at
lines 818..823 in that routine and will be executed when control reaches the sys_size
annotation logic inside the model_eqns handling.
2. Enable hyper_cleaning in the case input (so the module variable hyper_cleaning becomes
true by the time s_initialize_global_parameters_module runs). The module does not enforce
that hyper_cleaning implies mhd or multidimensional geometry anywhere in this file
(hyper_cleaning is an independent logical declared at line ~120).
3. With hyper_cleaning = .true. and mhd = .false. (or with a 1D run where n == 0), the
existing code at lines 818..823 will append psi_idx to sys_size even though the MHD state
(B_idx) or multidimensionality may not be present — changing the global sys_size and
therefore the expected state-vector layout.
4. The mismatch becomes observable when downstream code that assembles or solves the
system (or any IO/serialization expecting a specific ordering) assumes no psi state unless
MHD/multidimensionality is active. In practice this misconfiguration is realistic (user
sets hyper_cleaning without toggling mhd), so the current guardless insertion can produce
incorrect indexing. Guarding the insertion with ".and. mhd .and. n > 0" prevents adding
the psi field when MHD or multidimensional geometry is not active.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/post_process/m_global_parameters.fpp
**Line:** 818:818
**Comment:**
*Logic Error: The hyperbolic cleaning variable is appended to the system state vector whenever `hyper_cleaning` is true, regardless of whether MHD is enabled or the problem is multidimensional, which can create a state layout that does not match the assumptions of the MHD hyperbolic cleaning solver (implemented only for 2D/3D MHD), leading to inconsistent indexing and potential downstream runtime errors.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
High-level Suggestion
Refactor the new hardcoded initial conditions in Fortran files like
2dHardcodedIC.fppinto configurable input files for the test cases. This improves modularity and separates test configuration from core logic. [High-level, importance: 6]Solution Walkthrough:
Before:
After: