Skip to content

refactor: consolidate grid, stretch, and body force params into derived types#1432

Open
sbryngelson wants to merge 20 commits into
masterfrom
refactor/derived-types
Open

refactor: consolidate grid, stretch, and body force params into derived types#1432
sbryngelson wants to merge 20 commits into
masterfrom
refactor/derived-types

Conversation

@sbryngelson
Copy link
Copy Markdown
Member

@sbryngelson sbryngelson commented May 12, 2026

Summary

Consolidates several families of flat scalar parameters into Fortran derived types, reducing global variable count and making structure explicit. Implements items 1, 4, 5, 6, and 7 from issue #1427.

Grid coordinate arrays → type(grid_axis) (item 1)

  • x_cc, x_cb, x_cb_s (and y/z equivalents) → x%cc, x%cb, x%cb_s
  • dx, dy, dz min spacing → x%min_spacing, y%min_spacing, z%min_spacing
  • Analytical IC codegen updated to emit x%cc(i) instead of x_cc(i)

Boundary condition types → bc_dir_t + bc_side_t (item 4)

  • Introduced bc_dir_t to hold BC type (beg/end integers) separately from payload
  • Renamed bc_x%beg/endbc%x%beg/end (new compound bc_xyz_info struct)
  • Introduced bc_side_t with beg_side/end_side sub-structs, replacing 14 flat _in/_out/vb*/ve* fields:
    • pres_in/outbeg_side%pres / end_side%pres
    • vel_in/out(j)beg_side%vel(j) / end_side%vel(j)
    • vb1/2/3, ve1/2/3beg_side%vel_wall(1/2/3) / end_side%vel_wall(1/2/3)
    • alpha_rho_in(f), alpha_in(f)beg_side%alpha_rho(f), beg_side%alpha(f)
    • grcbc_in/out, grcbc_vel_outbeg_side%grcbc, end_side%grcbc, end_side%grcbc_vel
    • isothermal_in/out, Twall_in/outbeg_side%isothermal, beg_side%T_wall (and end_side equivalents)
  • Updated ~100 example/benchmark case.py files and golden test files
  • Fixed remove_higher_dimensional_keys to handle bc%y%beg-style keys for lower-dimensional cases

Body force parameters → type(body_force_t) (item 5)

  • Introduced body_force_t container holding type(body_force_axis) :: x, y, z
  • Replaced three separate bf_x, bf_y, bf_z variables with single bf struct
  • bf_x%k, bf_x%w, bf_x%p, bf_x%g, bf_x%enabled (and y/z) → bf%x%k, bf%x%w, etc.

IB dynamics → type(ib_dynamics_t) (item 6)

  • force_x/y/z, torque_x/y/z, vel_x/y/z, omega_x/y/z, angle_x/y/zforce%x/y/z, etc.

Grid stretching → type(bounds_info) (item 7)

  • x_a/x_b, y_a/y_b, z_a/z_bx_stretch%beg/end, y_stretch%beg/end, z_stretch%beg/end

Other

  • Pins and applies ffmt v0.4.0 formatting

Test plan

  • ./mfc.sh precheck passes (all 6 checks)
  • All three targets compile (pre_process, simulation, post_process)
  • Body force tests pass
  • Grid stretching tests pass
  • remove_higher_dimensional_keys fix verified
  • CI green

Closes part of #1427.

@qodo-code-review
Copy link
Copy Markdown
Contributor

ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 12, 2026

Codecov Report

❌ Patch coverage is 66.62791% with 287 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.94%. Comparing base (28863b1) to head (506c745).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/common/m_boundary_common.fpp 71.87% 19 Missing and 17 partials ⚠️
src/post_process/m_data_output.fpp 41.81% 32 Missing ⚠️
src/pre_process/m_icpp_patches.fpp 31.91% 31 Missing and 1 partial ⚠️
src/pre_process/m_start_up.fpp 40.00% 19 Missing and 5 partials ⚠️
src/common/m_mpi_common.fpp 29.03% 14 Missing and 8 partials ⚠️
src/simulation/m_bubbles_EL.fpp 30.00% 20 Missing and 1 partial ⚠️
src/pre_process/m_boundary_conditions.fpp 0.00% 16 Missing ⚠️
src/simulation/m_data_output.fpp 46.42% 15 Missing ⚠️
src/simulation/m_ibm.fpp 67.64% 11 Missing ⚠️
src/common/m_chemistry.fpp 52.38% 10 Missing ⚠️
... and 20 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1432      +/-   ##
==========================================
- Coverage   64.95%   64.94%   -0.02%     
==========================================
  Files          72       72              
  Lines       18879    18893      +14     
  Branches     1571     1584      +13     
==========================================
+ Hits        12263    12270       +7     
- Misses       5640     5646       +6     
- Partials      976      977       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Replace flat allocatable arrays x_cb/y_cb/z_cb, x_cc/y_cc/z_cc,
and dx/dy/dz with a derived type having .cb, .cc, and .spacing
components. All three executables (pre_process, simulation,
post_process) updated across 47 files.

Key design decisions:
- pre_process keeps scalar dx/dy/dz as minimum cell-width scalars;
  only x_cb and x_cc are folded into x%cb and x%cc
- OpenMP GPU target uses whole-struct declare target (x, y, z) since
  component-level declare target is invalid; OpenACC uses component-level
- 2dHardcodedIC.fpp wraps dx*dy in #ifdef MFC_PRE_PROCESS for the
  scalar vs per-cell context difference

Special variable collisions fixed:
- m_chemistry.fpp: local integer x/y/z -> cx/cy/cz
- m_weno.fpp: local real y(1:4) scratch -> ys
- m_viscous.fpp: local real dx(1:3) scratch -> ds
- m_ibm.fpp: local scalar dx/dy/dz -> dx_loc/dy_loc/dz_loc
- m_cbc.fpp: Fypp template d${XYZ}$ -> ${XYZ}$%spacing
Implements items 7 and 5 from issue #1427:
- x_a/x_b/y_a/y_b/z_a/z_b -> type(bounds_info) :: x_stretch, y_stretch, z_stretch
- bf_x/bf_y/bf_z + k_x/w_x/p_x/g_x (and y/z) -> type(body_force_axis) :: bf_x, bf_y, bf_z
@sbryngelson sbryngelson force-pushed the refactor/derived-types branch from 882206f to 3f9f382 Compare May 12, 2026 20:38
@sbryngelson sbryngelson force-pushed the refactor/derived-types branch from 15f3a64 to eb90ba8 Compare May 12, 2026 23:24
Remove module-level dx, dy, dz scalars from pre_process m_global_parameters.
Add min_spacing field to the grid_axis derived type so each axis carries its
own minimum cell width. Update all call sites in m_grid, m_start_up,
m_icpp_patches, m_mpi_common, and 2dHardcodedIC.
After reading grid data from files, compute and store min_spacing on each
axis in both serial and parallel paths. Matches the pre_process pattern
so min_spacing is consistent across all three executables.
Group the three directional boundary condition variables (bc_x, bc_y,
bc_z of type bc_dir_t) into a single bc_xyz_info struct accessed as
bc%x, bc%y, bc%z. Updates all Fortran source, Fypp macros, Python
toolchain, example cases, and documentation.
PR #1432 renamed bc_x%beg -> bc%x%beg. The remove_higher_dimensional_keys
helper only matched the old _y/_z separator style (.+_y, y_.+), so
bc%y%beg and bc%z%beg were not removed for lower-dimensional cases.
Add %{dim}% substring check to cover the new compound key format.
- m_thinc.fpp: take master's extended Fypp for-loop tuple (STENCIL_VAR,
  COORDS, X_BND/Y_BND/Z_BND), update CC_PRI x_cc/y_cc/z_cc -> x%cc/y%cc/z%cc
- m_rhs.fpp: take master's drop of 'dummy' workaround condition, keep bc%y%beg naming
- m_riemann_solvers.fpp: take master's unified Re_avg_rsx_vf indexing (j,k,l)
  for all cylindrical faces, update y_cb/y_cc -> y%cb/y%cc
Comment thread tests/failed_uuids.txt
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

Group the three body-force axis structs (bf_x, bf_y, bf_z) into a
single body_force_t container variable bf, matching the bc%x/y/z
compound naming pattern. Updates all Fortran source, Python toolchain,
examples, and docs.
- examples/3D_rayleigh_taylor/case.py: bf_y%* -> bf%y%*
- case_validator.py: check_body_forces uses bf%{dir}%enabled/k/w/p/g
- descriptions.py: regex pattern bf_([xyz]) -> bf%([xyz])%enabled
Replace flat rho_L/rho_R, pres_L/pres_R, E_L/E_R, H_L/H_R, gamma_L/R,
pi_inf_L/R, qv_L/R, c_L/R, T_L/R, MW_L/R, R_gas_L/R, Cp_L/R, Cv_L/R,
Gamm_L/R, G_L/R, Y_L/R, vel_L_rms/R_rms, vel_L_tmp/R_tmp, nbub_L/R,
ptilde_L/R with type(riemann_states) :: rho, pres, E, H, ... accessed
as rho%L / rho%R. Replace vel_L/vel_R with type(riemann_states_vec3) ::
vel accessed as vel%L(i) / vel%R(i).

HLLD already used this pattern; HLL, LF, and HLLC now match.
GPU private lists updated to name the struct once instead of _L and _R.
Cray ftn (CCE) requires globals accessed in GPU accelerator routines to
have ! declare directives. bc (type bc_xyz_info) was declared for
OpenMP but omitted from the OpenACC block, causing:

  ftn-7066 ftn: ERROR S_SLIP_WALL: Global in accelerator routine
  without declare -- bc

Add $:GPU_DECLARE(create='[bc]') to the MFC_OpenACC branch alongside
the existing OpenMP declaration.
…states structs

Extend the riemann_states refactor to cover wave speeds, intermediate
states, stress tensor, Reynolds numbers, and HLLD state vectors:

  s_L/R         -> s%L/R     (type riemann_states)
  Ms_L/R        -> Ms%L/R
  pres_SL/SR    -> pres_S%L/R
  flux_tau_L/R  -> flux_tau%L/R
  xi_L/R        -> xi%L/R    (HLLC)
  xi_L/R_m1     -> xi_m1%L/R (HLLC)
  pTot_L/R      -> pTot%L/R  (HLLD)
  rhoL/R_star   -> rho_star%L/R (HLLD)
  E_starL/R     -> E_star%L/R (HLLD)
  vL/R_star     -> v_star%L/R (HLLD)
  wL/R_star     -> w_star%L/R (HLLD)
  tau_e_L/R(i)  -> tau_e%L/R(i) (type riemann_states_arr6)
  Re_L/R(i)     -> Re%L/R(i)   (type riemann_states_arr2)
  xi_field_L/R  -> xi_field%L/R (riemann_states_vec3)
  U_L/R etc.    -> U%L/R etc.  (type riemann_states_arr7, HLLD)

Variable-size arrays (alpha_L/R, Ys_L/R, etc.) are left as-is since
they require allocatable components or Fypp-conditional sizing.

Also exclude fp-stability-logs/ and scripts/ from typos spell check.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants