diff --git a/src/common/m_global_parameters_common.fpp b/src/common/m_global_parameters_common.fpp index f436be4366..7ec6c0a6b4 100644 --- a/src/common/m_global_parameters_common.fpp +++ b/src/common/m_global_parameters_common.fpp @@ -419,6 +419,7 @@ contains muscl_order = dflt_int num_fluids = dflt_int igr = .false. + igr_order = dflt_int mhd = .false. relativity = .false. #:endif diff --git a/src/pre_process/m_mpi_proxy.fpp b/src/pre_process/m_mpi_proxy.fpp index 8e733765f3..a17122320f 100644 --- a/src/pre_process/m_mpi_proxy.fpp +++ b/src/pre_process/m_mpi_proxy.fpp @@ -80,7 +80,7 @@ contains ! manual: patch_icpp (complex members: alter_patch, sph_har_coeff, size() arrays) do i = 1, num_patches_max - #:for VAR in [ 'geometry', 'smooth_patch_id'] + #:for VAR in [ 'geometry', 'smooth_patch_id', 'hcid'] call MPI_BCAST(patch_icpp(i)%${VAR}$, 1, MPI_INTEGER, 0, MPI_COMM_WORLD, ierr) #:endfor @@ -91,7 +91,7 @@ contains #:for VAR in [ 'x_centroid', 'y_centroid', 'z_centroid', & & 'length_x', 'length_y', 'length_z', 'radius', 'epsilon', & & 'beta', 'smooth_coeff', 'rho', 'p0', 'm0', 'r0', 'v0', & - & 'pres', 'gamma', 'pi_inf', 'hcid', 'cv', 'qv', 'qvp', & + & 'pres', 'gamma', 'pi_inf', 'cv', 'qv', 'qvp', & & 'cf_val', 'Bx', 'By', 'Bz'] call MPI_BCAST(patch_icpp(i)%${VAR}$, 1, mpi_p, 0, MPI_COMM_WORLD, ierr) #:endfor diff --git a/toolchain/mfc/lint_source.py b/toolchain/mfc/lint_source.py index 59645ebd5a..535119207a 100644 --- a/toolchain/mfc/lint_source.py +++ b/toolchain/mfc/lint_source.py @@ -30,6 +30,9 @@ PRECISION_EXCLUDE_DIRS = {"syscheck"} PRECISION_EXCLUDE_PATTERNS = {"nvtx", "precision_select"} +# MPI proxy source directory -> params-registry target key +MPI_PROXY_TARGETS = {"pre_process": "pre", "simulation": "sim", "post_process": "post"} + def _is_comment_or_blank(stripped: str) -> bool: """True if stripped line is blank, a Fortran comment, or a Fypp directive.""" @@ -309,6 +312,131 @@ def check_junk_comments(repo_root: Path) -> list[str]: return errors +_BCAST_CALL_RE = re.compile(r"\bcall\s+MPI_BCAST\s*\(", re.IGNORECASE) +_FYPP_FOR_RE = re.compile(r"#:\s*for\s+(\w+)\s+in\s+\[") +_FYPP_ENDFOR_RE = re.compile(r"#:\s*endfor\b") +_PLACEHOLDER_RE = re.compile(r"\$\{(\w+)\}\$") +_IDENTIFIER_RE = re.compile(r"[A-Za-z_]\w*$") + + +def _first_bcast_argument(after_paren: str) -> str: + """Return the first argument of an MPI_BCAST call, given the text after '('.""" + depth = 0 + for pos, ch in enumerate(after_paren): + if ch == "(": + depth += 1 + elif ch == ")" and depth > 0: + depth -= 1 + elif ch in ",)" and depth == 0: + return after_paren[:pos] + return after_paren + + +def _expand_placeholders(arg: str, loops: dict[str, list[str]]) -> list[str]: + """Substitute Fypp ``${VAR}$`` placeholders with each active loop entry. + + Placeholders whose loop list has no quoted entries (e.g. numeric lists) + expand to nothing — those arguments cannot name a namelist scalar root. + """ + match = _PLACEHOLDER_RE.search(arg) + if not match: + return [arg] + expanded: list[str] = [] + for entry in loops.get(match.group(1), []): + expanded.extend(_expand_placeholders(arg[: match.start()] + entry + arg[match.end() :], loops)) + return expanded + + +def _normalize_bcast_root(candidate: str) -> str | None: + """Reduce a broadcast first argument to its root name, or None if not a top-level scalar. + + Struct members (containing '%') are legitimate manual residue; arguments + that are not plain identifiers after dropping the index part are skipped. + """ + candidate = candidate.strip() + if "%" in candidate or "$" in candidate: + return None + root = candidate.split("(", 1)[0].strip() + return root if _IDENTIFIER_RE.fullmatch(root) else None + + +def _extract_bcast_roots(lines: list[str]) -> list[tuple[int, str]]: + """Return (line_number, root_name) for every MPI_BCAST first argument. + + Resolves Fypp ``#:for VAR in ['a', 'b', ...]`` loop lists (including '&' + continuations and nested loops) so list-driven broadcasts are attributed + to the quoted variable names. + """ + roots: list[tuple[int, str]] = [] + loop_stack: list[tuple[str, list[str]]] = [] + + i = 0 + while i < len(lines): + stripped = lines[i].strip() + for_match = _FYPP_FOR_RE.match(stripped) + if for_match: + full = stripped + while full.rstrip().endswith("&") and i + 1 < len(lines): + i += 1 + full += " " + lines[i].strip() + entries = re.findall(r"['\"]([^'\"]*)['\"]", full[full.find("[") :]) + loop_stack.append((for_match.group(1), entries)) + elif _FYPP_ENDFOR_RE.match(stripped): + if loop_stack: + loop_stack.pop() + elif not stripped.startswith("!"): + call = _BCAST_CALL_RE.search(stripped) + if call: + first_arg = _first_bcast_argument(stripped[call.end() :]) + for candidate in _expand_placeholders(first_arg, dict(loop_stack)): + root = _normalize_bcast_root(candidate) + if root: + roots.append((i + 1, root)) + i += 1 + + return roots + + +def _registry_bcast_vars(repo_root: Path, target: str) -> set[str]: + """Names auto-broadcast by generated_bcast.fpp for one target. + + Derived from the generator itself so the lint and the generated code can + never disagree about which scalars are auto-broadcast: struct roots, + TYPED_DECLS, FORTRAN_ARRAY_DIMS, and per-target exclusions are already + removed by _classify_scalar_vars. + """ + toolchain_dir = str(repo_root / "toolchain") + if toolchain_dir not in sys.path: + sys.path.insert(0, toolchain_dir) + from mfc.params.generators.fortran_gen import _classify_scalar_vars + + return set().union(*_classify_scalar_vars(target)) + + +def check_manual_registry_bcasts(repo_root: Path) -> list[str]: + """Flag hand-written MPI_BCASTs of registry-bound namelist scalars. + + Those scalars are broadcast by the generated include (generated_bcast.fpp); + a manual copy in m_mpi_proxy.fpp is a duplicate broadcast, or a new + parameter bypassing the generator. Manual residue (computed variables and + struct members) is permitted. + """ + errors: list[str] = [] + + for dirname, target in MPI_PROXY_TARGETS.items(): + proxy = repo_root / SRC_DIR / dirname / "m_mpi_proxy.fpp" + if not proxy.exists(): + continue + auto_broadcast = _registry_bcast_vars(repo_root, target) + rel = proxy.relative_to(repo_root) + + for lineno, root in _extract_bcast_roots(proxy.read_text(encoding="utf-8").splitlines()): + if root in auto_broadcast: + errors.append(f" {rel}:{lineno} manual MPI_BCAST of registry-bound scalar '{root}' — it is auto-broadcast via generated_bcast.fpp; remove the manual copy") + + return errors + + def main(): repo_root = Path(__file__).resolve().parents[2] @@ -321,6 +449,7 @@ def main(): all_errors.extend(check_fypp_list_duplicates(repo_root)) all_errors.extend(check_duplicate_lines(repo_root)) all_errors.extend(check_hardcoded_byte_size(repo_root)) + all_errors.extend(check_manual_registry_bcasts(repo_root)) if all_errors: print("Source lint failed:") diff --git a/toolchain/mfc/params/generators/fortran_gen.py b/toolchain/mfc/params/generators/fortran_gen.py index 14e0f472a6..2c8f797123 100644 --- a/toolchain/mfc/params/generators/fortran_gen.py +++ b/toolchain/mfc/params/generators/fortran_gen.py @@ -243,16 +243,22 @@ def generate_case_opt_decls_fpp() -> str: # Post-process scalars that are namelist-bound but consumed on rank 0 only (reading/init). # Broadcasting them would be harmless but changes the existing call set, which we preserve. -_POST_BCAST_EXCLUDE = frozenset({"avg_state", "cfl_target", "igr_order", "num_bc_patches", "recon_type", "sigR"}) +# Namelist-bound names with no registry definition, verified rank-0-only consumers. +_NO_REGISTRY_ALLOWLIST = frozenset({"G"}) + +_POST_BCAST_EXCLUDE = frozenset({"avg_state", "cfl_target", "num_bc_patches", "sigR"}) # TYPED_DECLS entries kept entirely in the manual residue: their member-broadcast structure # is irregular (non-uniform subsets, size() arrays, nested loops, complex guards). -def _mpi_type_for(ptype: ParamType) -> str: - """Return the Fortran MPI type constant for a ParamType.""" +def _mpi_type_for(ptype: ParamType, storage_precision: bool = False) -> str: + """Return the Fortran MPI type constant for a ParamType. + + REAL parameters declared real(stp) (storage_precision=True) must broadcast + with mpi_io_p; mpi_p would silently mismatch under --mixed precision.""" if ptype in _REAL_TYPES: - return "mpi_p" + return "mpi_io_p" if storage_precision else "mpi_p" if ptype in (ParamType.INT, ParamType.ANALYTIC_INT): return "MPI_INTEGER" if ptype == ParamType.LOG: @@ -296,13 +302,19 @@ def _classify_scalar_vars(target: str) -> Tuple[List[str], List[str], List[str], pdef = REGISTRY.all_params.get(name) if pdef is None: - continue # no registry entry — skip (e.g. 'G' in post is rank-0-only) + if name in _NO_REGISTRY_ALLOWLIST: + continue + raise ValueError( + f"namelist var {name!r} ({target}) has no registry entry. " + f"Register it in definitions.py or add it to _NO_REGISTRY_ALLOWLIST; " + f"a silent skip here means a silently missing broadcast." + ) if target == "sim" and name in CASE_OPT_PARAMS: case_opt_vars.append(name) continue - mpi_type = _mpi_type_for(pdef.param_type) + mpi_type = _mpi_type_for(pdef.param_type, getattr(pdef, "storage_precision", False)) if mpi_type == "MPI_INTEGER": int_vars.append(name) elif mpi_type == "MPI_LOGICAL": @@ -403,7 +415,7 @@ def _emit_fortran_array_dims(lines: List[str], target: str) -> None: pdef = REGISTRY.all_params.get(member_key) if pdef is None: raise ValueError(f"No registry entry for {member_key!r} (needed for FORTRAN_ARRAY_DIMS broadcast)") - mpi_type = _mpi_type_for(pdef.param_type) + mpi_type = _mpi_type_for(pdef.param_type, getattr(pdef, "storage_precision", False)) lines.append(f" call MPI_BCAST({name}(1), {dim}, {mpi_type}, 0, MPI_COMM_WORLD, ierr)") diff --git a/toolchain/mfc/test_lint_source.py b/toolchain/mfc/test_lint_source.py new file mode 100644 index 0000000000..6c788543f6 --- /dev/null +++ b/toolchain/mfc/test_lint_source.py @@ -0,0 +1,75 @@ +"""Tests for the manual registry-bound broadcast lint in lint_source.py.""" + +from mfc.lint_source import _extract_bcast_roots, check_manual_registry_bcasts + +BCAST_TAIL = ", 1, mpi_p, 0, MPI_COMM_WORLD, ierr)" + + +def test_extract_direct_call_root(): + lines = [f" call MPI_BCAST(m_glb{BCAST_TAIL}"] + assert _extract_bcast_roots(lines) == [(1, "m_glb")] + + +def test_extract_indexed_argument_keeps_root(): + lines = [f" call MPI_BCAST(phi_rn(1){BCAST_TAIL}"] + assert _extract_bcast_roots(lines) == [(1, "phi_rn")] + + +def test_extract_fypp_list_with_continuation(): + lines = [ + " #:for VAR in [ 'weno_eps', 'teno_CT', &", + " & 'pref']", + " call MPI_BCAST(${VAR}$" + BCAST_TAIL, + " #:endfor", + ] + assert _extract_bcast_roots(lines) == [(3, "weno_eps"), (3, "teno_CT"), (3, "pref")] + + +def test_struct_members_and_loop_indices_skipped(): + lines = [ + " #:for VAR in [ 'bc_x%beg', 'bc_x%end']", + " call MPI_BCAST(${VAR}$" + BCAST_TAIL, + " #:endfor", + " #:for DIM in ['x', 'y', 'z']", + " #:for DIR in [1, 2, 3]", + " call MPI_BCAST(bc_${DIM}$%vb${DIR}$" + BCAST_TAIL, + " #:endfor", + " #:endfor", + f" call MPI_BCAST(patch_ib(i)%geometry{BCAST_TAIL}", + " #:for VAR in ['c', 'p', 't', 'm']", + " call MPI_BCAST(ib_airfoil(i)%${VAR}$" + BCAST_TAIL, + " #:endfor", + ] + assert _extract_bcast_roots(lines) == [] + + +def _write_proxy(tmp_path, target_dir: str, body: str): + proxy_dir = tmp_path / "src" / target_dir + proxy_dir.mkdir(parents=True) + (proxy_dir / "m_mpi_proxy.fpp").write_text(body, encoding="utf-8") + + +def test_manual_broadcast_of_registry_scalar_is_flagged(tmp_path): + _write_proxy(tmp_path, "simulation", f" call MPI_BCAST(rhoref{BCAST_TAIL}\n") + + errors = check_manual_registry_bcasts(tmp_path) + assert len(errors) == 1 + assert "manual MPI_BCAST of registry-bound scalar 'rhoref'" in errors[0] + assert "m_mpi_proxy.fpp:1" in errors[0] + + +def test_manual_residue_is_clean(tmp_path): + body = "\n".join( + [ + f" call MPI_BCAST(m_glb{BCAST_TAIL}", + f" call MPI_BCAST(cfl_dt{BCAST_TAIL}", + " #:for VAR in [ 'bc_x%beg', 'bc_x%end']", + " call MPI_BCAST(${VAR}$" + BCAST_TAIL, + " #:endfor", + f" call MPI_BCAST(patch_icpp(i)%geometry{BCAST_TAIL}", + "", + ] + ) + _write_proxy(tmp_path, "simulation", body) + + assert check_manual_registry_bcasts(tmp_path) == []