Remove airfoil and stl paramters

[danieljvickers]

Description

The patch_ib array has become bloated as many variable have been added to this struct, growing each element to 962 bytes. Several of these variables are unused except in niche cases. Two such examples are the STL models and airfoils. Particularly the STL model is responsible for 492 bytes of this usage (over half).

Since we are not MPI communicating the patch_ib elements regularly, it is important to try to optimize their size. This will improve communication time as well as likely providing a noticeable performance benefit. This was done by creating separate smaller data structures that the patches will call out to that actually contain airfoil and STL information. This will make minor modifications to the case file, which other users should be made aware of.

Fixes #1451

Type of change

• Refactor

Testing

Ran test suite.

Checklist

• I added or updated tests for new behavior
• I updated documentation if user-facing behavior changed

[github-actions]

Claude Code Review

Head SHA: 0d27a5e

Files changed:

• 25
• src/common/m_model.fpp
• src/common/m_constants.fpp
• src/common/m_derived_types.fpp
• src/pre_process/m_check_ib_patches.fpp
• src/simulation/m_compute_levelset.fpp
• src/simulation/m_ib_patches.fpp
• src/simulation/m_global_parameters.fpp
• toolchain/mfc/params/definitions.py
• toolchain/mfc/case_validator.py

Findings

1. Unguarded print in s_instantiate_STL_models — MPI correctness regression
src/common/m_model.fpp, new line 992 (inside do stl_id = 1, num_stl_models):

print *, " * Reading model: " // trim(stl_models(stl_id)%model_filepath)

The old code had if (proc_rank == 0) print *, " * Reading model: ...". The guard was dropped in the refactor. Every MPI rank will now print this message, flooding output on any parallel run. All surrounding prints in the same function retain if (proc_rank == 0) guards (lines 1005-1006, 1023-1024, 1031-1035, 1053).

---

2. Potential out-of-bounds array access in airfoil IB checker — correctness bug
src/pre_process/m_check_ib_patches.fpp, lines 110–114 (and identically 127–131):

@:PROHIBIT(n == 0 .or. p > 0 .or. patch_ib(patch_id)%airfoil_id <= 0 &
           & .or. ib_airfoil(patch_ib(patch_id)%airfoil_id)%c <= 0._wp &

When airfoil_id = 0 (the default), Fortran's .or. does not guarantee short-circuit evaluation (F2003 §7.2.4), so ib_airfoil(0)%c may be evaluated before the airfoil_id <= 0 sub-expression triggers. ib_airfoil is dimension(num_ib_airfoils_max) (1-based, size 5), so index 0 is out of bounds. The model-checker in the same file already uses the correct pattern — split the bounds check into its own @:PROHIBIT before the content checks, matching what s_check_model_ib_patch_geometry does for model_id.

---

3. Wrong Fortran constant in definitions.py inflates patch_ib registry to 2,050,000 entries
toolchain/mfc/params/definitions.py:

-NIB = _fc("num_ib_patches_max_namelist", 50000)  # patch_ib namelist limit
+NIB = _fc("num_ib_patches_max", 50000)  # patch_ib (Fortran array bound)

Both constants exist in src/common/m_constants.fpp: num_ib_patches_max_namelist = 54000 and num_ib_patches_max = 2050000. The _fc() call now resolves to 2050000 (not the fallback 50000) because the named constant is present. NIB is used as max_index for the patch_ib indexed family. Raising it from ~54000 to 2,050,000 means the Python parameter registry will attempt to enumerate ~2 million patch_ib(i)%* entries, causing severe toolchain performance regressions (case validation, ./mfc.sh params, ./mfc.sh lint). The constant name should remain "num_ib_patches_max_namelist".

[codecov]

Codecov Report

❌ Patch coverage is 66.39004% with 81 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.80%. Comparing base (2d0d2fd) to head (257e52d).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
src/common/m_model.fpp	53.73%	17 Missing and 14 partials ⚠️
src/simulation/m_ib_patches.fpp	72.72%	16 Missing and 8 partials ⚠️
src/simulation/m_compute_levelset.fpp	33.33%	18 Missing ⚠️
src/pre_process/m_check_ib_patches.fpp	28.57%	5 Missing ⚠️
src/simulation/m_ibm.fpp	50.00%	1 Missing and 1 partial ⚠️
src/simulation/m_start_up.fpp	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1524      +/-   ##
==========================================
+ Coverage   60.63%   60.80%   +0.16%     
==========================================
  Files          73       73              
  Lines       20219    20199      -20     
  Branches     2937     2932       -5     
==========================================
+ Hits        12259    12281      +22     
+ Misses       5972     5932      -40     
+ Partials     1988     1986       -2     

☔ 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.

[danieljvickers]

@sbryngelson approval for benchmark?

[danieljvickers]

AI commanets have been addressed, btw

[wilfonba]

@danieljvickers Benchmark results are not looking good, even with NVHPC. A quick look through the diff didn't give me any idea why either.

[danieljvickers]

@wilfonba the only test with slow down was Phoenix openACC. It showed 20% slowdowns in pre-process and post-process, and 40% in simulation. Slow down is even for cases without IBM enabled. But Phoenix openMP did not show slow down in any execution times. Because pre-process and post-process are CPU only, that must mean that the binaries being run in the ACC test are identical to the MP test. I think this indicates that the slowdown is just some acute/random failure on the node and not a real slowdown.

I will rerun this for safety, but nothing about the results lead me to think this PR introduces an NVHPC openACC slowdown yet.

[danieljvickers]

Reran the case and got back good execution/grind times. It looks like the previous run was just randomly slow.

[sbryngelson]

Case Pre Process Simulation Post Process
──────────────────────────────────────────────────────────────────────────────
igr Exec: 0.80 Exec: 0.79 & Grind: Exec: 0.78
0.52
ibm Exec: 0.79 Exec: 0.74 & Grind: Exec: 0.87
0.49
viscous_weno5_sgb_aco… Exec: 0.76 Exec: 0.76 & Grind: Exec: 0.85
0.63
hypo_hll Exec: 0.79 Exec: 0.78 & Grind: Exec: 0.81
0.58
5eq_rk3_weno3_hllc Exec: 0.75 Exec: 0.78 & Grind: Exec: 0.84
0.65

[danieljvickers]

@sbryngelson I thought I checked all cases last night and the only one out of place was Phoenix OpenACC, which reran fine. What is that from?

Edit, looking at the old run: https://github.com/MFlowCode/MFC/actions/runs/26841117410/job/79221540422
I got OpenACC and OpenMP mixed up in my message. my previous comment still holds true that if we see no regression in the CPU case and/or we see no regression in the other GPU code, then regression in pre-process and post-process in other cases is likely just noise in the measurement. I expect it to rerun fine still.

[danieljvickers]

@sbryngelson The new benchmark times are all good. Ready to merge.