Skip to content

Commit 6dec86a

Browse files
authored
Fix three pre-existing multi-rank broadcast defects; deregister dead parameters (#1553)
1 parent f86b337 commit 6dec86a

6 files changed

Lines changed: 149 additions & 41 deletions

File tree

src/post_process/m_mpi_proxy.fpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,14 @@ contains
8585
call MPI_BCAST(${VAR}$, 1, MPI_LOGICAL, 0, MPI_COMM_WORLD, ierr)
8686
#:endfor
8787

88+
! wall-velocity members consumed by s_slip_wall/s_no_slip_wall on all ranks
89+
#:for DIM in ['x', 'y', 'z']
90+
#:for DIR in [1, 2, 3]
91+
call MPI_BCAST(bc_${DIM}$%vb${DIR}$, 1, mpi_p, 0, MPI_COMM_WORLD, ierr)
92+
call MPI_BCAST(bc_${DIM}$%ve${DIR}$, 1, mpi_p, 0, MPI_COMM_WORLD, ierr)
93+
#:endfor
94+
#:endfor
95+
8896
! manual: cfl_dt (runtime-computed logical), bc_io (BC-file existence)
8997
call MPI_BCAST(cfl_dt, 1, MPI_LOGICAL, 0, MPI_COMM_WORLD, ierr)
9098
call MPI_BCAST(bc_io, 1, MPI_LOGICAL, 0, MPI_COMM_WORLD, ierr)

src/pre_process/m_mpi_proxy.fpp

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,28 @@ contains
3939
call MPI_BCAST(${VAR}$, 1, MPI_LOGICAL, 0, MPI_COMM_WORLD, ierr)
4040
#:endfor
4141

42-
! manual: bc_x/y/z and domain bounds (registered as struct members, not in NAMELIST_VARS)
42+
! manual: domain bounds and wall temperatures (REAL struct members, not in NAMELIST_VARS)
4343
#:for VAR in [ 'x_domain%beg', 'x_domain%end', 'y_domain%beg', &
4444
& 'y_domain%end', 'z_domain%beg', 'z_domain%end', &
45-
& 'bc_x%beg', 'bc_x%end', 'bc_y%beg', 'bc_y%end', &
46-
& 'bc_z%beg', 'bc_z%end', 'bc_x%Twall_in', 'bc_x%Twall_out', &
45+
& 'bc_x%Twall_in', 'bc_x%Twall_out', &
4746
& 'bc_y%Twall_in', 'bc_y%Twall_out', 'bc_z%Twall_in', &
4847
& 'bc_z%Twall_out']
4948
call MPI_BCAST(${VAR}$, 1, mpi_p, 0, MPI_COMM_WORLD, ierr)
5049
#:endfor
5150

51+
! manual: BC type codes (INTEGER struct members)
52+
#:for VAR in [ 'bc_x%beg', 'bc_x%end', 'bc_y%beg', 'bc_y%end', 'bc_z%beg', 'bc_z%end']
53+
call MPI_BCAST(${VAR}$, 1, MPI_INTEGER, 0, MPI_COMM_WORLD, ierr)
54+
#:endfor
55+
56+
! wall-velocity members consumed by s_slip_wall/s_no_slip_wall on all ranks
57+
#:for DIM in ['x', 'y', 'z']
58+
#:for DIR in [1, 2, 3]
59+
call MPI_BCAST(bc_${DIM}$%vb${DIR}$, 1, mpi_p, 0, MPI_COMM_WORLD, ierr)
60+
call MPI_BCAST(bc_${DIM}$%ve${DIR}$, 1, mpi_p, 0, MPI_COMM_WORLD, ierr)
61+
#:endfor
62+
#:endfor
63+
5264
! manual: cfl_dt (runtime-computed logical), bc_io (BC-file existence)
5365
call MPI_BCAST(cfl_dt, 1, MPI_LOGICAL, 0, MPI_COMM_WORLD, ierr)
5466
call MPI_BCAST(bc_io, 1, MPI_LOGICAL, 0, MPI_COMM_WORLD, ierr)

toolchain/mfc/params/definitions.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -842,14 +842,13 @@ def _load():
842842
_r(f"{px}sph_har_coeff({ll},{mm})", REAL)
843843

844844
# fluid_pp (10 fluids)
845+
# Members present in physical_parameters: gamma, pi_inf, Re, cv, qv, qvp, G.
846+
# mul0/ss/pv/gamma_v/M_v/mu_v/k_v/cp_v/D_v were removed from the Fortran type
847+
# by upstream #1085/#1093 — they must NOT be registered (namelist read would crash).
845848
for f in range(1, NF + 1):
846849
px = f"fluid_pp({f})%"
847850
for a, sym in [("gamma", r"\f$\gamma_k\f$"), ("pi_inf", r"\f$\pi_{\infty,k}\f$"), ("cv", r"\f$c_{v,k}\f$"), ("qv", r"\f$q_{v,k}\f$"), ("qvp", r"\f$q'_{v,k}\f$")]:
848851
_r(f"{px}{a}", REAL, math=sym)
849-
_r(f"{px}mul0", REAL, {"viscosity"}, math=r"\f$\mu_{l,k}\f$")
850-
_r(f"{px}ss", REAL, {"surface_tension"}, math=r"\f$\sigma_k\f$")
851-
for a in ["pv", "gamma_v", "M_v", "mu_v", "k_v", "cp_v", "D_v"]:
852-
_r(f"{px}{a}", REAL, {"bubbles"})
853852
_r(f"{px}G", REAL, {"elasticity"}, math=r"\f$G_k\f$")
854853
_r(f"{px}Re(1)", REAL, {"viscosity"}, math=r"\f$\mathrm{Re}_k\f$ (shear)")
855854
_r(f"{px}Re(2)", REAL, {"viscosity"}, math=r"\f$\mathrm{Re}_k\f$ (bulk)")
@@ -1049,11 +1048,16 @@ def _load():
10491048
_r(f"simplex_params%perturb_vel_offset({d},{j})", REAL)
10501049

10511050
# lag_params (Lagrangian bubbles)
1051+
# Members present in bubbles_lagrange_parameters: solver_approach, cluster_type,
1052+
# pressure_corrector, smooth_type, heatTransfer_model, massTransfer_model,
1053+
# write_bubbles, write_bubbles_stats, nBubs_glb, epsilonb, charwidth, valmaxvoid.
1054+
# T0/Thost/c0/rho0/x0 were removed from the Fortran type by upstream #1085/#1093
1055+
# — they must NOT be registered (namelist read would crash).
10521056
for a in ["heatTransfer_model", "massTransfer_model", "pressure_corrector", "write_bubbles", "write_bubbles_stats"]:
10531057
_r(f"lag_params%{a}", LOG, {"bubbles"})
10541058
for a in ["solver_approach", "cluster_type", "smooth_type", "nBubs_glb"]:
10551059
_r(f"lag_params%{a}", INT, {"bubbles"})
1056-
for a in ["epsilonb", "valmaxvoid", "charwidth", "c0", "rho0", "T0", "x0", "Thost"]:
1060+
for a in ["epsilonb", "valmaxvoid", "charwidth"]:
10571061
_r(f"lag_params%{a}", REAL, {"bubbles"})
10581062

10591063
# chem_params

toolchain/mfc/params/descriptions.py

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -323,15 +323,6 @@
323323
(r"fluid_pp\((\d+)\)%qv", "Heat of formation for fluid {0}"),
324324
(r"fluid_pp\((\d+)\)%qvp", "Heat of formation prime for fluid {0}"),
325325
(r"fluid_pp\((\d+)\)%Re\((\d+)\)", "Reynolds number component {1} for fluid {0}"),
326-
(r"fluid_pp\((\d+)\)%mul0", "Reference liquid viscosity for fluid {0}"),
327-
(r"fluid_pp\((\d+)\)%ss", "Surface tension for fluid {0}"),
328-
(r"fluid_pp\((\d+)\)%pv", "Vapor pressure for fluid {0}"),
329-
(r"fluid_pp\((\d+)\)%gamma_v", "Specific heat ratio of vapor phase for fluid {0}"),
330-
(r"fluid_pp\((\d+)\)%M_v", "Molecular weight of vapor phase for fluid {0}"),
331-
(r"fluid_pp\((\d+)\)%mu_v", "Viscosity of vapor phase for fluid {0}"),
332-
(r"fluid_pp\((\d+)\)%k_v", "Thermal conductivity of vapor phase for fluid {0}"),
333-
(r"fluid_pp\((\d+)\)%cp_v", "Specific heat capacity (const. pressure) of vapor for fluid {0}"),
334-
(r"fluid_pp\((\d+)\)%D_v", "Vapor mass diffusivity for fluid {0}"),
335326
(r"fluid_pp\((\d+)\)%non_newtonian", "Enable Herschel-Bulkley non-Newtonian viscosity for fluid {0}"),
336327
(r"fluid_pp\((\d+)\)%K", "HB consistency index for fluid {0}"),
337328
(r"fluid_pp\((\d+)\)%nn", "HB flow behavior index for fluid {0}"),
@@ -500,11 +491,6 @@
500491
(r"lag_params%epsilonb", "Standard deviation scaling for Gaussian smoothing"),
501492
(r"lag_params%charwidth", "Domain virtual depth for 2D simulations"),
502493
(r"lag_params%valmaxvoid", "Maximum permitted void fraction"),
503-
(r"lag_params%T0", "Initial bubble temperature"),
504-
(r"lag_params%Thost", "Host fluid temperature"),
505-
(r"lag_params%c0", "Initial sound speed"),
506-
(r"lag_params%rho0", "Initial density"),
507-
(r"lag_params%x0", "Initial bubble position"),
508494
(r"lag_params%(\w+)", "Lagrangian tracking parameter: {0}"),
509495
# chem_params patterns - specific fields first
510496
(r"chem_params%diffusion", "Enable species diffusion for chemistry"),

toolchain/mfc/params/generators/fortran_gen.py

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,11 @@ def generate_case_opt_decls_fpp() -> str:
235235
_STRUCT_ROOTS = frozenset({"bc_x", "bc_y", "bc_z", "x_domain", "y_domain", "z_domain", "x_output", "y_output", "z_output"})
236236

237237
# Variables excluded from broadcast generation (derived post-broadcast or non-namelist).
238-
_BCAST_EXCLUDE = frozenset({"muscl_eps"})
238+
# muscl_eps was previously excluded here on the assumption that it was derived
239+
# post-broadcast, but the derivation only fires under f_is_default(muscl_eps),
240+
# and default values are assigned on rank 0 only. Every multi-rank MUSCL run
241+
# therefore had rank-divergent muscl_eps. Removed from exclusion to fix the bug.
242+
_BCAST_EXCLUDE: frozenset = frozenset()
239243

240244
# Post-process scalars that are namelist-bound but consumed on rank 0 only (reading/init).
241245
# Broadcasting them would be harmless but changes the existing call set, which we preserve.
@@ -318,15 +322,19 @@ def _emit_bcast_group(lines: List[str], vars_list: List[str], mpi_type: str) ->
318322
def _emit_fluid_pp(lines: List[str], target: str) -> None:
319323
"""Emit the fluid_pp(i) member-loop broadcast block.
320324
321-
Members broadcast: the 13 REAL members (EOS core plus the Herschel-Bulkley
322-
non-Newtonian set) and the LOGICAL non_newtonian flag, matching the manual
323-
lists this generator replaces. Sim additionally: Re(1) with count=2.
325+
Walks the registry for every fluid_pp member, emitting the MPI datatype that
326+
matches each member's registered ParamType (the Herschel-Bulkley merge added
327+
a LOGICAL member, so the datatype must come from the registry, not be assumed
328+
REAL). Sim additionally broadcasts Re(1) with count=2 (kept sim-only to
329+
preserve the historical call set; pre/post never consumed Re).
330+
mul0/ss/pv/gamma_v/M_v/mu_v/k_v/cp_v/D_v were removed from the Fortran type by
331+
upstream #1085/#1093 and are no longer registered.
324332
"""
325-
fp_real_members = ["gamma", "pi_inf", "G", "cv", "qv", "qvp", "K", "nn", "tau0", "hb_m", "mu_min", "mu_max", "mu_bulk"]
333+
fp_members = sorted(k.split("%", 1)[1] for k in REGISTRY.all_params if k.startswith("fluid_pp(1)%") and not k.startswith("fluid_pp(1)%Re("))
326334
lines.append(" do i = 1, num_fluids_max")
327-
for mem in fp_real_members:
328-
lines.append(f" call MPI_BCAST(fluid_pp(i)%{mem}, 1, mpi_p, 0, MPI_COMM_WORLD, ierr)")
329-
lines.append(" call MPI_BCAST(fluid_pp(i)%non_newtonian, 1, MPI_LOGICAL, 0, MPI_COMM_WORLD, ierr)")
335+
for mem in fp_members:
336+
ptype = REGISTRY.all_params[f"fluid_pp(1)%{mem}"].param_type
337+
lines.append(f" call MPI_BCAST(fluid_pp(i)%{mem}, 1, {_mpi_type_for(ptype)}, 0, MPI_COMM_WORLD, ierr)")
330338
if target == "sim":
331339
lines.append(" call MPI_BCAST(fluid_pp(i)%Re(1), 2, mpi_p, 0, MPI_COMM_WORLD, ierr)")
332340
lines.append(" end do")
@@ -347,14 +355,13 @@ def _emit_bub_pp(lines: List[str]) -> None:
347355
def _emit_lag_params(lines: List[str]) -> None:
348356
"""Emit the lag_params member broadcast block (sim-only, under bubbles_lagrange guard).
349357
350-
Subset of lag_params members that are actually broadcast: the fields that appear in the
351-
existing simulation m_mpi_proxy.fpp lag_params block. The registry has additional
352-
members (T0, Thost, c0, rho0, x0) that are not broadcast (rank-0-only).
358+
All registered lag_params members are broadcast. T0/Thost/c0/rho0/x0 were removed
359+
from the Fortran type by upstream #1085/#1093 and are no longer in the registry.
353360
"""
354-
# Hardcoded broadcast subset — matches the existing sim m_mpi_proxy exactly.
355-
lag_log = ["heatTransfer_model", "massTransfer_model", "pressure_corrector", "write_bubbles", "write_bubbles_stats"]
356-
lag_int = ["cluster_type", "nBubs_glb", "smooth_type", "solver_approach"]
357-
lag_real = ["charwidth", "epsilonb", "valmaxvoid"]
361+
# Walk the registry for lag_params members, split by type.
362+
lag_log = sorted(k.split("%", 1)[1] for k in REGISTRY.all_params if k.startswith("lag_params%") and REGISTRY.all_params[k].param_type == ParamType.LOG)
363+
lag_int = sorted(k.split("%", 1)[1] for k in REGISTRY.all_params if k.startswith("lag_params%") and REGISTRY.all_params[k].param_type in (ParamType.INT, ParamType.ANALYTIC_INT))
364+
lag_real = sorted(k.split("%", 1)[1] for k in REGISTRY.all_params if k.startswith("lag_params%") and REGISTRY.all_params[k].param_type in _REAL_TYPES)
358365
lines.append(" if (bubbles_lagrange) then")
359366
for mem in sorted(lag_log):
360367
lines.append(f" call MPI_BCAST(lag_params%{mem}, 1, MPI_LOGICAL, 0, MPI_COMM_WORLD, ierr)")

toolchain/mfc/params_tests/test_fortran_gen.py

Lines changed: 95 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -387,8 +387,9 @@ def test_generate_bcast_fpp_case_opt_guard_sim():
387387
assert "num_fluids" in guard_body
388388
assert "mapped_weno" in guard_body
389389

390-
# muscl_eps must NOT appear anywhere (it is excluded — derived post-broadcast)
391-
assert "muscl_eps" not in out
390+
# muscl_eps IS inside the case-opt guard (it is sim-only, non-CASE_OPT_PARAM,
391+
# so it appears in the real-scalars section, which is outside the case-opt block)
392+
assert "call MPI_BCAST(muscl_eps, 1, mpi_p, 0, MPI_COMM_WORLD, ierr)" in out
392393

393394

394395
def test_generate_bcast_fpp_case_opt_not_in_pre_post():
@@ -504,8 +505,6 @@ def test_generate_bcast_fpp_excludes_manual_residue():
504505
assert "m_glb" not in out, f"{target}: m_glb should be manual"
505506
assert "n_glb" not in out, f"{target}: n_glb should be manual"
506507
assert "p_glb" not in out, f"{target}: p_glb should be manual"
507-
# muscl_eps is excluded (derived post-broadcast)
508-
assert "muscl_eps" not in out, f"{target}: muscl_eps should be excluded"
509508

510509
sim = generate_bcast_fpp("sim")
511510
# shear_stress, bulk_stress, bodyForces are derived (non-namelist)
@@ -530,3 +529,95 @@ def test_generate_bcast_fpp_bad_target():
530529

531530
with pytest.raises(ValueError, match="Unknown target"):
532531
generate_bcast_fpp("bad")
532+
533+
534+
def test_generate_bcast_fpp_muscl_eps_now_broadcast():
535+
"""muscl_eps is broadcast for sim (latent-bug fix: derivation is rank-0-only).
536+
537+
Previously excluded via _BCAST_EXCLUDE; every multi-rank MUSCL run had
538+
rank-divergent muscl_eps because f_is_default() only fires on rank 0.
539+
"""
540+
from mfc.params.generators.fortran_gen import generate_bcast_fpp
541+
542+
sim = generate_bcast_fpp("sim")
543+
assert "call MPI_BCAST(muscl_eps, 1, mpi_p, 0, MPI_COMM_WORLD, ierr)" in sim
544+
545+
# muscl_eps is sim-only; must not appear in pre/post
546+
pre = generate_bcast_fpp("pre")
547+
post = generate_bcast_fpp("post")
548+
assert "muscl_eps" not in pre
549+
assert "muscl_eps" not in post
550+
551+
552+
def test_generate_bcast_fpp_fluid_pp_registry_walk():
553+
"""fluid_pp emitter walks registry; no dead members (mul0/ss/pv/gamma_v/M_v/mu_v/k_v/cp_v/D_v removed upstream).
554+
555+
Re(1) count=2 is sim-only; all other registered REAL members appear in all three
556+
targets.
557+
"""
558+
from mfc.params.generators.fortran_gen import generate_bcast_fpp
559+
560+
sim = generate_bcast_fpp("sim")
561+
pre = generate_bcast_fpp("pre")
562+
post = generate_bcast_fpp("post")
563+
564+
# All registered members appear in every target
565+
for mem in ("gamma", "pi_inf", "cv", "qv", "qvp", "G"):
566+
for out, t in [(sim, "sim"), (pre, "pre"), (post, "post")]:
567+
assert f"fluid_pp(i)%{mem}" in out, f"{t}: fluid_pp(i)%{mem} missing"
568+
569+
# Re(1) count=2 is sim-only
570+
assert "fluid_pp(i)%Re(1)" in sim
571+
assert "fluid_pp(i)%Re(1)" not in pre
572+
assert "fluid_pp(i)%Re(1)" not in post
573+
574+
# Dead members must not appear
575+
for dead in ("mul0", "ss", "pv", "gamma_v", "M_v", "mu_v", "k_v", "cp_v", "D_v"):
576+
for out, t in [(sim, "sim"), (pre, "pre"), (post, "post")]:
577+
assert f"fluid_pp(i)%{dead}" not in out, f"{t}: dead member fluid_pp(i)%{dead} present"
578+
579+
580+
def test_generate_bcast_fpp_fluid_pp_member_datatypes():
581+
# The HB merge added a LOGICAL fluid_pp member; datatypes must come from the
582+
# registry, not be assumed REAL.
583+
from mfc.params.generators.fortran_gen import generate_bcast_fpp
584+
585+
sim = generate_bcast_fpp("sim")
586+
assert "call MPI_BCAST(fluid_pp(i)%non_newtonian, 1, MPI_LOGICAL," in sim
587+
assert "call MPI_BCAST(fluid_pp(i)%tau0, 1, mpi_p," in sim
588+
assert "non_newtonian, 1, mpi_p" not in sim
589+
590+
591+
def test_generate_bcast_fpp_lag_params_registry_walk():
592+
"""lag_params emitter walks registry; no dead members (T0/Thost/c0/rho0/x0 removed upstream)."""
593+
from mfc.params.generators.fortran_gen import generate_bcast_fpp
594+
595+
sim = generate_bcast_fpp("sim")
596+
597+
# All registered members must appear
598+
for mem in ("solver_approach", "cluster_type", "smooth_type", "nBubs_glb"):
599+
assert f"lag_params%{mem}" in sim, f"lag_params%{mem} missing from sim"
600+
for mem in ("heatTransfer_model", "massTransfer_model", "pressure_corrector", "write_bubbles", "write_bubbles_stats"):
601+
assert f"lag_params%{mem}" in sim, f"lag_params%{mem} missing from sim"
602+
for mem in ("epsilonb", "charwidth", "valmaxvoid"):
603+
assert f"lag_params%{mem}" in sim, f"lag_params%{mem} missing from sim"
604+
605+
# Dead members must not appear
606+
for dead in ("T0", "Thost", "c0", "rho0", "x0"):
607+
assert f"lag_params%{dead}" not in sim, f"dead member lag_params%{dead} present in sim"
608+
609+
610+
def test_mpi_proxy_residue_pins_wall_velocity_and_bc_datatypes():
611+
"""The vb/ve wall-velocity broadcasts and integer BC datatypes live in
612+
hand-written residue (not codegen); pin them so an edit or merge conflict
613+
that drops them fails loudly."""
614+
import pathlib
615+
616+
root = pathlib.Path(__file__).resolve().parents[3]
617+
for target in ("pre_process", "post_process"):
618+
src = (root / "src" / target / "m_mpi_proxy.fpp").read_text()
619+
assert "bc_${DIM}$%vb${DIR}$" in src, f"{target}: vb broadcasts missing"
620+
assert "bc_${DIM}$%ve${DIR}$" in src, f"{target}: ve broadcasts missing"
621+
assert "'bc_x%beg', 'bc_x%end', 'bc_y%beg', 'bc_y%end', 'bc_z%beg', 'bc_z%end']" in src
622+
seg = src.split("'bc_z%beg', 'bc_z%end']", 1)[1]
623+
assert "MPI_INTEGER" in seg.split("#:endfor")[0], f"{target}: BC codes not MPI_INTEGER"

0 commit comments

Comments
 (0)