-
Notifications
You must be signed in to change notification settings - Fork 133
MTHINC Interface Compression #1303
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
base: master
Are you sure you want to change the base?
Changes from all commits
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -425,7 +425,7 @@ See @ref equations "Equations" for the mathematical models these parameters cont | |||||
| | `mp_weno` | Logical | Monotonicity preserving WENO | | ||||||
| | `muscl_order` | Integer | MUSCL order [1,2] | | ||||||
| | `muscl_lim` | Integer | MUSCL Slope Limiter: [1] minmod; [2] monotonized central; [3] Van Albada; [4] Van Leer; [5] SUPERBEE | | ||||||
| | `int_comp` | Logical | THINC Interface Compression | | ||||||
| | `int_comp` | Integer | Interface Compression [1] THINC [2] MTHINC | | ||||||
|
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. Document all three valid states for The table entry only lists values [1] and [2], but according to the PR objectives, 📝 Suggested documentation fix-| `int_comp` | Integer | Interface Compression [1] THINC [2] MTHINC |
+| `int_comp` | Integer | Interface Compression: [0] Off; [1] THINC; [2] MTHINC |📝 Committable suggestion
Suggested change
|
||||||
| | `ic_eps` | Real | Interface compression threshold (default: 1e-4) | | ||||||
| | `ic_beta` | Real | Interface compression sharpness parameter (default: 1.6) | | ||||||
| | `riemann_solver` | Integer | Riemann solver algorithm: [1] HLL*; [2] HLLC; [3] Exact*; [4] HLLD (only for MHD) | | ||||||
|
|
@@ -519,7 +519,7 @@ It is recommended to set `weno_eps` to $10^{-6}$ for WENO-JS, and to $10^{-40}$ | |||||
| - `muscl_lim` specifies the slope limiter that is used in 2nd order MUSCL Reconstruction by an integer from 1 through 5. | ||||||
| `muscl_lim = 1`, `2`, `3`, `4`, and `5` correspond to minmod, monotonized central, Van Albada, Van Leer, and SUPERBEE, respectively. | ||||||
|
|
||||||
| - `int_comp` activates interface compression using THINC used in MUSCL Reconstruction, with control parameters (`ic_eps`, and `ic_beta`). | ||||||
| - `int_comp` activates interface compression using [1] THINC or [2] MTHINC used in variable reconstruction, with control parameters (`ic_eps`, and `ic_beta`). | ||||||
|
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. Clarify all three states and the behavior when The description states " 📝 Suggested documentation improvement-- `int_comp` activates interface compression using [1] THINC or [2] MTHINC used in variable reconstruction, with control parameters (`ic_eps`, and `ic_beta`).
+- `int_comp` controls interface compression in variable reconstruction with control parameters (`ic_eps`, and `ic_beta`). `int_comp = 0` disables interface compression; `int_comp = 1` enables THINC compression; `int_comp = 2` enables MTHINC compression.Based on learnings, modifications to public parameters affect downstream users. This parameter changed from boolean to integer semantics, so complete documentation of all states is essential for users migrating from the previous boolean interface. |
||||||
|
|
||||||
| - `riemann_solver` specifies the choice of the Riemann solver that is used in simulation by an integer from 1 through 4. | ||||||
| `riemann_solver = 1`, `2`, and `3` correspond to HLL, HLLC, and Exact Riemann solver, respectively (\cite Toro09). | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -174,9 +174,9 @@ module m_global_parameters | |
| logical :: mixture_err !< Mixture properties correction | ||
| logical :: hypoelasticity !< hypoelasticity modeling | ||
| logical :: hyperelasticity !< hyperelasticity modeling | ||
| logical :: int_comp !< THINC interface compression | ||
| integer :: int_comp !< Interface compression: 0=off, 1=THINC, 2=MTHINC | ||
| real(wp) :: ic_eps !< THINC Epsilon to compress on surface cells | ||
| real(wp) :: ic_beta !< THINC Sharpness Parameter | ||
| real(wp) :: ic_beta !< THINC/MTHINC Sharpness Parameter | ||
|
Comment on lines
+177
to
+179
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. 🧩 Analysis chain🏁 Script executed: # First, check the GPU_DECLARE and GPU_UPDATE lists in m_global_parameters.fpp
cat -n src/simulation/m_global_parameters.fpp | grep -A 5 -B 5 "GPU_DECLARE\|GPU_UPDATE" | head -100Repository: MFlowCode/MFC Length of output: 4902 🏁 Script executed: # Check what's at lines 177-179 and 627-629 in m_global_parameters.fpp
sed -n '175,181p' src/simulation/m_global_parameters.fpp
sed -n '625,631p' src/simulation/m_global_parameters.fppRepository: MFlowCode/MFC Length of output: 753 🏁 Script executed: # Search for uses of int_comp, ic_eps, ic_beta in m_thinc.fpp
rg "int_comp|ic_eps|ic_beta" src/simulation/m_thinc.fpp -n --context 3Repository: MFlowCode/MFC Length of output: 6402 🏁 Script executed: # Check if s_compute_mthinc_normals and s_apply_thinc_compression contain GPU kernels
# Look for GPU_DATA, GPU_KERNEL, or !$acc pragmas in m_thinc.fpp around the relevant subroutines
rg "s_compute_mthinc_normals|s_apply_thinc_compression" src/simulation/m_thinc.fpp -A 50 | head -150Repository: MFlowCode/MFC Length of output: 6505 🏁 Script executed: # Check if the code that uses int_comp, ic_eps, ic_beta is within GPU regions
# Look for GPU_DATA or !$acc markers near the usage lines
sed -n '220,500p' src/simulation/m_thinc.fpp | grep -E "GPU_DATA|GPU_KERNEL|!.acc|int_comp|ic_eps|ic_beta"Repository: MFlowCode/MFC Length of output: 1546 🏁 Script executed: # Get fuller context: check if there are GPU regions at all in m_thinc.fpp
rg "GPU_DATA|GPU_KERNEL|!.acc" src/simulation/m_thinc.fpp -nRepository: MFlowCode/MFC Length of output: 39 🏁 Script executed: # Check if int_comp, ic_eps, ic_beta appear in any GPU_DECLARE or GPU_UPDATE statements elsewhere in m_global_parameters.fpp
rg "int_comp|ic_eps|ic_beta" src/simulation/m_global_parameters.fppRepository: MFlowCode/MFC Length of output: 364 🏁 Script executed: # Verify the complete context: check where int_comp, ic_eps, ic_beta are declared and look for nearby GPU_DECLARE
sed -n '170,190p' src/simulation/m_global_parameters.fppRepository: MFlowCode/MFC Length of output: 1380 🏁 Script executed: # Check if these variables need GPU_DECLARE by looking at the learning guidance and similar scalar globals
# Look for patterns: declaration followed by GPU_DECLARE for nearby scalars
sed -n '213,225p' src/simulation/m_global_parameters.fppRepository: MFlowCode/MFC Length of output: 792 Add These three variables are read inside GPU kernels in |
||
| integer :: hyper_model !< hyperelasticity solver algorithm | ||
| logical :: elasticity !< elasticity modeling, true for hyper or hypo | ||
| logical, parameter :: chemistry = .${chemistry}$. !< Chemistry modeling | ||
|
|
@@ -624,7 +624,7 @@ contains | |
| ptgalpha_eps = dflt_real | ||
| hypoelasticity = .false. | ||
| hyperelasticity = .false. | ||
| int_comp = .false. | ||
| int_comp = 0 | ||
| ic_eps = dflt_ic_eps | ||
| ic_beta = dflt_ic_beta | ||
| elasticity = .false. | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -56,6 +56,8 @@ module m_rhs | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| use m_igr | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| use m_thinc | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| use m_pressure_relaxation | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| implicit none | ||||||||||||||||||||||
|
|
@@ -744,6 +746,12 @@ contains | |||||||||||||||||||||
| call nvtxEndRange | ||||||||||||||||||||||
| end if | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if (int_comp == 2 .and. n > 0) then | ||||||||||||||||||||||
| call nvtxStartRange("RHS-COMPRESSION-NORMALS") | ||||||||||||||||||||||
| call s_compute_mthinc_normals(q_prim_qp%vf) | ||||||||||||||||||||||
| call nvtxEndRange | ||||||||||||||||||||||
| end if | ||||||||||||||||||||||
|
Comment on lines
+749
to
+753
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. Use the same THINC enablement guard here as in startup.
Minimal fix- if (int_comp == 2 .and. n > 0) then
+ if (int_comp == 2 .and. n > 0 .and. (.not. igr .or. dummy)) then
call nvtxStartRange("RHS-COMPRESSION-NORMALS")
call s_compute_mthinc_normals(q_prim_qp%vf)
call nvtxEndRange
end if📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ! Dimensional Splitting Loop | ||||||||||||||||||||||
| do id = 1, num_dims | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
@@ -780,7 +788,7 @@ contains | |||||||||||||||||||||
| if ((.not. igr) .or. dummy) then! Finite volume solve | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ! Reconstructing Primitive/Conservative Variables | ||||||||||||||||||||||
| call nvtxStartRange("RHS-WENO") | ||||||||||||||||||||||
| call nvtxStartRange("RHS-RECONSTRUCTION") | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if (.not. surface_tension) then | ||||||||||||||||||||||
| if (all(Re_size == 0)) then | ||||||||||||||||||||||
|
|
@@ -1923,8 +1931,21 @@ contains | |||||||||||||||||||||
| recon_dir, & | ||||||||||||||||||||||
| is1, is2, is3) | ||||||||||||||||||||||
| end if | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if (int_comp > 0 .and. iv%beg <= advxb .and. iv%end >= advxe) then | ||||||||||||||||||||||
| ! Only run interface compression when the volume fractions are | ||||||||||||||||||||||
| ! in the reconstructed variables | ||||||||||||||||||||||
| call nvtxStartRange("RHS-RECONSTRUCTION-COMPRESSION") | ||||||||||||||||||||||
| call s_thinc_compression(q_prim_qp%vf, & | ||||||||||||||||||||||
| vL_x, vL_y, vL_z, & | ||||||||||||||||||||||
| vR_x, vR_y, vR_z, & | ||||||||||||||||||||||
| recon_dir, & | ||||||||||||||||||||||
| is1, is2, is3) | ||||||||||||||||||||||
| call nvtxEndRange() | ||||||||||||||||||||||
| end if | ||||||||||||||||||||||
| end if | ||||||||||||||||||||||
| #:endfor | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| end subroutine s_reconstruct_cell_boundary_values | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| !> @brief Performs first-order (piecewise constant) reconstruction of left and right cell-boundary values. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ module m_start_up | |
| !! schemes for spatial reconstruction of variables | ||
|
|
||
| use m_muscl !< Monotonic Upstream-centered (MUSCL) | ||
| use m_thinc !< THINC/MTHINC interface compression | ||
| !! schemes for convservation laws | ||
|
|
||
| use m_riemann_solvers !< Exact and approximate Riemann problem solvers | ||
|
|
@@ -1116,6 +1117,7 @@ contains | |
| elseif (recon_type == MUSCL_TYPE) then | ||
| call s_initialize_muscl_module() | ||
| end if | ||
| if (int_comp > 0) call s_initialize_thinc_module() | ||
|
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. Mirror THINC finalization with the initialization predicate. Initialization happens inside As per coding guidelines, Also applies to: 1304-1304 |
||
| call s_initialize_cbc_module() | ||
| call s_initialize_riemann_solvers_module() | ||
| end if | ||
|
|
@@ -1299,6 +1301,7 @@ contains | |
| elseif (recon_type == MUSCL_TYPE) then | ||
| call s_finalize_muscl_module() | ||
| end if | ||
| if (int_comp > 0) call s_finalize_thinc_module() | ||
| end if | ||
| call s_finalize_variables_conversion_module() | ||
| if (grid_geometry == 3) call s_finalize_fftw_module | ||
|
|
||
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.
The parameter table entry for
int_compwas updated to an integer but no longer documents the0=offvalue (it currently only lists[1] THINC [2] MTHINC). This can confuse users and contradicts other docs/tooling that treat0as the default/off value. Please update the table text to include0explicitly.