figr: IGR-only miniapp cleanup and feature port#1
Merged
sbryngelson merged 34 commits intomainfrom Apr 4, 2026
Merged
Conversation
… types
Remove all legacy MFC infrastructure unrelated to IGR:
- Bubble dynamics and QBMM: ~35 variables (bubbles_euler, polytropic,
polydisperse, qbmm, nb, Eu, Ca, Web, Re_inv, weight, R0, ptil,
poly_sigma, pi_fac, mom_sp, mom_3d, and all physical bubble params),
their GPU_DECLARE/UPDATE entries, MPI broadcasts, and dead branches
in m_data_output.fpp
- Probe and integral I/O: s_open/write/close_{com,probe}_files,
associated globals (integral_wrt, num_probes, num_integrals,
probe(:), integral(:)), MPI broadcasts, and namelist entries
across all three targets
- m_derived_variables module (simulation): deleted entirely;
s_compute/initialize_derived_variables had empty bodies,
helpers were never called
- Unused quadrature-point fields: q_cons_qp, q_prim_qp (allocated
and deallocated but never accessed in IGR builds)
- Dead index variables: n_idx, gamma_idx, pi_inf_idx,
internalEnergies_idx, intxb, intxe, stress_idx, xi_idx removed
from simulation, pre_process, and post_process m_global_parameters
- s_assign_patch_mixture_primitive_variables (pre_process): procedure
pointer is hardwired to the species version; mixture path unreachable
- s_convert_mixture_to_mixture_variables (common): model_eqns==1 path,
never dispatched to in IGR-only build; also removes last uses of
gamma_idx/pi_inf_idx from common code
- integral_parameters type, num_probes_max, nnode (QBMM) from
m_derived_types and m_constants
- s_initialize_internal_energy_equations (simulation): exported but
never called; referenced invalid equation indices in IGR-only build
- num_probe_ts (simulation m_time_steppers): private integer never
assigned or read
Review Summary by QodoRemove dead code: bubble/QBMM, probe/integral I/O, unused indices
WalkthroughsDescription• Remove ~35 dead bubble/QBMM variables and infrastructure • Delete probe/integral I/O subroutines and related globals • Remove unused index variables and derived types • Delete entire m_derived_variables module with empty bodies • Clean up unused quadrature-point fields and MPI broadcasts Diagramflowchart LR
A["Dead Code Removal"] --> B["Bubble/QBMM Variables"]
A --> C["Probe/Integral I/O"]
A --> D["Index Variables"]
A --> E["Derived Types"]
B --> F["~35 variables removed"]
C --> G["s_open/write/close subroutines"]
D --> H["gamma_idx, pi_inf_idx, etc."]
E --> I["integral_parameters type"]
File Changes1. src/common/m_constants.fpp
|
Code Review by Qodo
1. model_eqns silently ignored
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
206
to
214
| integer :: sys_size !< Number of unknowns in system of eqns. | ||
| type(int_bounds_info) :: cont_idx !< Indexes of first & last continuity eqns. | ||
| type(int_bounds_info) :: mom_idx !< Indexes of first & last momentum eqns. | ||
| integer :: E_idx !< Index of energy equation | ||
| integer :: n_idx !< Index of number density | ||
| type(int_bounds_info) :: adv_idx !< Indexes of first & last advection eqns. | ||
| type(int_bounds_info) :: internalEnergies_idx !< Indexes of first & last internal energy eqns. | ||
| integer :: alf_idx !< Index of void fraction | ||
| integer :: gamma_idx !< Index of specific heat ratio func. eqn. | ||
| integer :: pi_inf_idx !< Index of liquid stiffness func. eqn. | ||
| integer :: E_idx !< Index of energy equation | ||
| type(int_bounds_info) :: adv_idx !< Indexes of first & last advection eqns. | ||
| integer :: alf_idx !< Index of void fraction | ||
| !> @} | ||
| $:GPU_DECLARE(create='[sys_size, E_idx, n_idx, alf_idx, gamma_idx]') | ||
| $:GPU_DECLARE(create='[pi_inf_idx]') | ||
| $:GPU_DECLARE(create='[sys_size, E_idx, alf_idx]') | ||
|
|
There was a problem hiding this comment.
1. Model_eqns silently ignored 🐞 Bug ≡ Correctness
The PR removes non-model_eqns==2 state indices/conversion paths, but model_eqns is still read from input and never validated, while global indexing is hard-wired for the volume-fraction model. Users can set model_eqns to an unsupported value and proceed with incorrect state-vector interpretation/patch validation, leading to wrong results or hard-to-debug failures.
Agent Prompt
## Issue description
Non-volume-fraction model support was removed (e.g., gamma/pi_inf indexing and mixture-to-mixture conversion), but `model_eqns` is still accepted from input and broadcast without validation. The code then unconditionally configures the system as the volume-fraction model, so unsupported `model_eqns` values can silently produce incorrect state-vector interpretation and/or skip relevant validation.
## Issue Context
Both pre_process and simulation now appear to be IGR-only / volume-fraction-only, but they still expose `model_eqns` as a user input. If only `model_eqns == 2` is supported, the code should fail fast with a clear message (or remove the input entirely) to prevent silent misconfiguration.
## Fix Focus Areas
- Add an explicit check/prohibition in simulation input validation:
- src/simulation/m_checker.fpp[20-60]
- Add an explicit check/prohibition in pre_process input validation (currently empty):
- src/pre_process/m_checker.fpp[17-26]
- (Optional but recommended) also enforce close to where indexing is set, to guard against future call sites bypassing checkers:
- src/simulation/m_global_parameters.fpp[430-460]
- src/pre_process/m_global_parameters.fpp[315-340]
- (Optional) update/remove `model_eqns` from namelists to avoid implying configurability:
- src/simulation/m_start_up.fpp[67-85]
- src/pre_process/m_start_up.fpp[75-82]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
- Delete m_perturbation.fpp and m_simplex_noise.fpp (entire modules) - Remove all perturbation/mixlayer/simplex/elliptic_smoothing parameters from pre_process global_parameters, namelist, MPI broadcasts, and dispatch - Remove geometry 13 (2D Fourier modal patch) from m_icpp_patches, including fourier_cos/fourier_sin fields in ic_patch_parameters and modal_* helpers - Remove fluid_pp%G (elasticity shear modulus) from all targets: derived types, variables_conversion (Gs_vc, G_K, G args throughout), mpi_proxy broadcasts, and global_parameters defaults - Remove f_compute_filtered_dtheta dead stub from m_sim_helpers - Remove fourier_rings and max_2d_fourier_modes constants; remove PRNG constants (modulus/multiplier/increment/amplifier/decimal_trim) used only by m_perturbation
- Rename mfc.sh -> figr.sh, mfc.bat -> figr.bat - Rename toolchain/mfc/ -> toolchain/figr/ (Python package directory) - Update all Python imports: from mfc -> from figr - Rename MFCException -> FigrException, MFC_CLI_SCHEMA -> FIGR_CLI_SCHEMA - Update CMakeLists.txt project name: MFC -> figr - Update all documentation, GitHub workflows, shell scripts, and comments - Update pyproject.toml, ruff.toml, .ffmt.toml, homebrew formula - Preprocessor defines (MFC_MPI, MFC_SIMULATION, etc.) left as-is: they are compile-time feature flags used in hundreds of ifdef guards
- Remove MFC_MPI CMake option; MPI is now unconditionally linked - Strip all #ifdef MFC_MPI / #ifndef MFC_MPI guards from 19 Fortran source files, keeping only the MPI code paths - Remove --no-mpi CLI flag (drop mpi field from MFCConfig dataclass) - Remove no-mpi test matrix entry from CI - Remove no-MPI runtime validation check
…tants - Remove pres_field type, pb_ts/mv_ts/rhs_pb/rhs_mv (zero-size bubble pressure arrays threaded through time_steppers -> rhs -> boundary_common -> mpi_common for interface compatibility only) - Remove field_position, species_parameters types (never instantiated) - Remove ic_patch_parameters%Y field (species mass fractions, never read) - Remove dead constants: Chem_Tolerance, dflt_T_guess, Strang splitting constants (threshold_*/scale_*/small_guess/dflt_adap_dt_*), relativity_cons_to_prim_max_iter - Remove dead helpers: s_swap, f_cross, s_print_2D_array (public, 0 callers) - Remove dead locals: T, rhoYks in pre_process/m_data_output
Implements the packer package (toolchain/figr/packer/) with: - pack.py: Pack/PackEntry classes, pack() to read Fortran binary .dat files from D/ directory, load()/save() for golden.txt text format - tol.py: Tolerance class and compare() for element-wise comparison with combined absolute+relative tolerance Handles both Fortran unformatted sequential (with record markers) and raw MPI-IO binary formats, auto-detecting double vs single precision.
- Remove ./figr.sh spelling, lint, precheck commands (lint.sh and precheck.sh were already missing; spelling is overkill for a miniapp) - Remove .githooks/pre-commit and auto-install hook logic from figr.sh - Remove shell completions bootstrap (completions.sh) - Remove .typos.toml config - Keep ./figr.sh format (still useful) - Update CLAUDE.md and common-pitfalls.md docs
…I routines - Remove weno_Re_flux, null_weights from simulation global parameters, defaults, and MPI broadcasts (declared but never read) - Remove pres_in/pres_out, vel_in/vel_out, grcbc_in/grcbc_out/grcbc_vel_out from int_bounds_info type (broadcast and GPU-updated but never read) - Remove s_mpi_allreduce_vectors_sum, s_mpi_allreduce_integer_sum from m_mpi_common (public, zero callers) - Remove dead rdma_mpi PROHIBIT check from m_checker
- Remove .github/scripts/ (15 benchmark/SLURM scripts, unreferenced by any workflow) - Remove .github/workflows/common/ (4 dead helper scripts for removed workflows) - Remove .github/workflows/frontier/ and frontier_amd/ (2 dead Frontier CI scripts) - Remove .lychee.toml (link checker config, no workflow exists) - Remove .github/file-filter.yml (paths-filter config, unreferenced by test.yml) - Remove .github/.dockerignore (no Docker workflow exists) - Clean .fortlsrc: remove references to dead files (m_qbmm, hipfort, cutensorex, etc.) - Fix MFC branding in python.sh, cmake.sh, format.sh
- Remove mp_weno, weno_avg, wa_flg, mixture_err, weno_eps, teno_CT, teno, wenojs, mapped_weno, wenoz, wenoz_q, muscl_order, muscl_lim, muscl_polyn from simulation m_global_parameters (declarations, defaults, GPU_DECLARE/UPDATE, MPI broadcasts) - All were declared/broadcast/GPU'd but never read in any computation - Remove s_compute_fd_divergence from m_finite_differences (0 callers)
- Remove mp_weno, weno_avg, wa_flg, mixture_err, weno_eps, teno_CT, teno, wenojs, mapped_weno, wenoz, wenoz_q, muscl_order, muscl_lim, muscl_polyn from simulation (all dead: never read in computation) - Remove s_compute_fd_divergence from m_finite_differences (0 callers) - Fix template MPI launch: pass mpi=True to templates so all executables (including syscheck) are launched via mpirun - Fix phoenix.mako: mfc.sh -> figr.sh
The original figr test cases used periodic BCs with a sharp discontinuity, which produced NaN in the CFL computation. Replace with the upstream MFC IGR test configuration: - 3-patch Sod shock tube (graduated pressure/density) - Ghost extrapolation BCs (bc=-3) instead of periodic (bc=-1) - 1-fluid, 2-fluid, viscous, and 3D variants - IGR orders 3 and 5, Jacobi and Gauss-Seidel solvers All 8 tests pass. UUIDs now match upstream MFC for equivalent cases. Remove 6 stale golden file directories from old test definitions.
- Remove --mpi from CI test commands (flag no longer exists, MPI is always on) - Remove mpi matrix variable from workflow (only mpi config remains) - Remove gcov option from MFCConfig, build.py, and CMakeLists.txt (coverage instrumentation, LTO skipping, FYPP line markers) - Remove fastmath option from MFCConfig, build.py, and CMakeLists.txt (-gpu=fastmath for NVHPC GPUs)
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
- Remove entire MFC_DOCUMENTATION block from CMakeLists.txt (250 lines of doxygen/docs generation infrastructure) - Strip doxygen comment markers (!> @file, !! @brief, @param, @name, @{/@}, @cond/@endcond) from all 45 Fortran source files - Remove dead WENO/MUSCL/TENO variables from simulation that were accidentally re-added by a stash operation: weno_eps, teno_CT, mp_weno, weno_avg, wa_flg, mixture_err, wenojs, mapped_weno, wenoz, teno, wenoz_q, muscl_order, muscl_lim, muscl_polyn - Remove cf_val field from ic_patch_parameters (zero usages) - Remove s_compute_fd_divergence from m_finite_differences (0 callers)
Port fix from MFlowCode/MFC#1350: the ~25-30GB NVHPC cuda_multi Docker images exceed GitHub runner disk space when pulled via the container: directive (which runs before any steps). Fix: remove container: directive, add explicit steps to free disk space (remove dotnet/android/ghc/boost/chromium), then docker pull + docker run -d + docker exec for NVHPC jobs. Non-NVHPC jobs are unchanged. Also fix: fetch main instead of master for coverage diff.
- Fix all 13 .mako templates: mfc.sh -> figr.sh in module load commands - Fix helpers.mako: MFC prologue/epilogue/case -> figr - Fix all toolchain Python: CLI descriptions, help text, error messages, case template generator, test runner, lock file, clean command - Rename --mfc CLI arg to --figr (case.py template interface) - Fix .gitignore, .fortlsrc, CMakeLists.txt, README.md, .claude/rules - Fix toolchain/modules, toolchain/dependencies/CMakeLists.txt - Remove stale docs/superpowers/ planning documents
Global rename of all 327 occurrences of MFC_ preprocessor defines to FIGR_ across 50 files: Fortran sources (.fpp/.f90), CMakeLists.txt, Python toolchain, Mako templates, .fortlsrc. Includes: FIGR_MPI, FIGR_SIMULATION, FIGR_PRE_PROCESS, FIGR_POST_PROCESS, FIGR_OpenACC, FIGR_OpenMP, FIGR_GPU, FIGR_DEBUG, FIGR_SINGLE_PRECISION, FIGR_MIXED_PRECISION, FIGR_CASE_OPTIMIZATION, FIGR_SYSCHECK, FIGR_Unified, and all other FIGR_* defines.
The simulation writes D/*.dat files as text (x [y [z]] value per line), not Fortran unformatted binary. The packer was trying struct.unpack on text data, producing empty entries. Golden files had filenames but no values, causing all cross-compiler tests to fail with length mismatches. Fix: read .dat files as text, parse space-separated numbers, extract only the value column (discard coordinate columns). Matches the upstream MFC packer behavior. Regenerated all 8 golden files with actual data.
- Remove fltr_dtheta unused variable from m_sim_helpers (2 declarations) - Remove axisymmetric grid branches from m_grid (grid_geometry always 1, Cartesian-only in IGR build) — ~30 lines of dead cylindrical grid code - Unwrap model_eqns==2 conditionals in post_process (always true in IGR): m_data_output, m_start_up, m_check_patches - Remove model_eqns from GPU_DECLARE/GPU_UPDATE (never read on device) - Fix stale GPU_UPDATE that still referenced removed vars (mixture_err, mp_weno, weno_eps, teno_CT)
- Remove 14 unused constants from m_constants: dflt_char, small_alf, verysmall, pathlen_max, dflt_vcfl_dt, and 9 unused BC enums (BC_RIEMANN_EXTRAP, BC_CHAR_SLIP_WALL, BC_CHAR_NR_SUB_BUFFER, BC_CHAR_NR_SUB_INFLOW, BC_CHAR_NR_SUB_OUTFLOW, BC_CHAR_FF_SUB_OUTFLOW, BC_CHAR_CP_SUB_OUTFLOW, BC_CHAR_SUP_INFLOW, BC_NULL) - Remove shear_stress, bulk_stress from simulation m_global_parameters (set during init, GPU'd, but never read in any computation)
The modules file exported MFC_CUDA_CC but CMakeLists.txt looks for FIGR_CUDA_CC after the preprocessor define rename. Without the correct env var, GPU compute capabilities were never passed to nvfortran. Note: the GPU build still has an nvlink undefined reference issue that predates our changes (present on the initial commit too). The device code for ! routine functions in m_sim_helpers is not being generated properly. This needs further investigation with an interactive GPU session.
- Fix toolchain/cmake/configuration.cmake.in: MFC_* -> FIGR_* CMake vars - Fix toolchain/bootstrap/cmake.sh: MFC_CMAKE_MIN_* -> FIGR_CMAKE_MIN_* - Rename Python classes: MFCConfig -> FigrConfig, MFCTarget -> FigrTarget, MFCInputFile -> FigrInputFile, MFCLockData -> FigrLockData, MFCPrinter -> FigrPrinter - Fix .claude/rules/ docs: MFC_* -> FIGR_* preprocessor define names
- Add num_vels to GPU_DECLARE(create=...) in simulation m_global_parameters. Without this, nvfortran could not generate device code for ! routine seq functions that reference num_vels, causing nvlink 'Undefined reference' errors for s_compute_enthalpy and other GPU routines. - Rename remaining mfc references in Python toolchain: mfc_config -> figr_config, get_mfc_root -> get_figr_root, MFCLockData -> FigrLockData, etc. - Delete stale __pycache__ directories
… branches - Remove input_bubbles_lagrange, create_input_lagrange, copy_input_lagrange functions from test/case.py (dead bubble infrastructure) - Remove bubbles_lagrange conditional from test run/cleanup paths - Simplify compute_tolerance: remove all dead feature branches (hypoelasticity, mixlayer, qbmm, bubbles, acoustic, mhd, axisymmetric, cylindrical) - Remove unused shutil import - Remove stale bubble example entries from .gitignore
- Add double_mach parameter with shock variables (xshock, cf, Mach, pshock, rhoshock, velshock, rho0_dm, p0_dm, u0_dm, v0_dm, xr_dm, theta_dm, gam_dm) across all 3 targets - Add 3 new hardcoded IC cases in 2dHardcodedIC.fpp: 283 (IGR isentropic vortex), 284 (2D IGR Riemann test), 285 (2D IGR double Mach reflection) - Add symm_2d_blending/blend subroutines in m_icpp_patches.fpp - Add double_mach boundary conditions in m_boundary_common.fpp (shock tracking, ghost cell overrides, reflective wall) - Add 3 new example cases: 2D_IGR_double_mach, 2D_IGR_isentropicvortex, 2D_IGR_riemann_test - Update 3D_IGR_TaylorGreenVortex example (N=127, Re=10000, V0=1.25*C0) - All 8 existing tests pass
sbryngelson
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Comprehensive cleanup of the IGR-only miniapp derived from MFC, plus feature port from MFlowCode/MFC#1346.
Rename MFC → figr
mfc.sh→figr.sh,mfc.bat→figr.bat,toolchain/mfc/→toolchain/figr/MFC_preprocessor defines toFIGR_MFCConfig→FigrConfig, etc.)Dead code removal (~3,000+ lines)
m_perturbation.fpp,m_simplex_noise.fpp)fluid_pp%G(elasticity), 2D Fourier patches,pres_field/pb_ts/mv_tsSimplifications
--no-mpioption, stripped#ifdef MFC_MPIguards)gcovandfastmathbuild optionsmodel_eqns == 2conditionalsgrid_geometryalways Cartesian)Bug fixes
num_velstoGPU_DECLARE(nvfortran device code generation)MFC_CUDA_CC→FIGR_CUDA_CCin modules.datfiles as text (not binary)mpirun--mpiflag removed from workflow, NVHPC disk space fix from MFC#1350Feature port (MFC#1346)
2D_IGR_double_mach,2D_IGR_isentropicvortex,2D_IGR_riemann_test3D_IGR_TaylorGreenVortexexampleTest plan