Size species arrays from the generated mechanism on AMD - #1918
Merged
Merged
Conversation
Generic (non-case-optimized) amdflang builds declared every species array
with a fixed AMD_NUM_SPECIES_MAX = 60 bound, and sys_size arrays with
AMD_SYS_SIZE_MAX = 70, because the species count was not known when the
guards were written. The toolchain now writes
build/include/<target>/thermochem.fpp with m_thermochem's species count
(NUM_SPECIES) and whether chemistry is on (CHEMISTRY). Species arrays are
declared dimension(${NUM_SPECIES}$) on every backend, so the USING_AMD
species branches collapse to one declaration and carry the mechanism's
exact count. Chemistry pins num_fluids to 1, so the sys_size fallback
becomes 10 + NUM_SPECIES there; other builds keep 70. The species-count
check goes away with the bound.
Fypp searches the including file's directory before -I paths, so the
default thermochem.fpp for toolchain-less Fypp runs lives in
src/common/include/defaults/, searched last.
Done with Claude Code.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1918 +/- ##
=======================================
Coverage 62.82% 62.82%
=======================================
Files 86 86
Lines 22394 22394
Branches 3305 3305
=======================================
Hits 14070 14070
Misses 6071 6071
Partials 2253 2253 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Conflict resolution:
- m_ibm: keep master's alpha_rho_GP, drop the per-branch Ys_IP in favor of
the single ${NUM_SPECIES}$ declaration
- input.py: compute sol before master's syscheck guard; thermochem.fpp
needs it for every target
- test_thermochem fypp helper: search src/common/include/defaults last, as
cmake/Fypp.cmake does, for the default thermochem.fpp
Its sandiego.yaml mechanism is the only non-h2o2 one in the suite, so it forces a second chemistry build of every target just for this case, which takes too long to compile. It stays in the example skip list.
Lines of Code
|
sbryngelson
added a commit
to rocfire11/MFC-tlj
that referenced
this pull request
Oct 1, 2026
Conflicts with MFlowCode#1918, which sizes species locals at ${NUM_SPECIES}$ instead of AMD_NUM_SPECIES_MAX / num_species: the reacting-surface arrays in s_ibm_correct_state join master's single Ys_IP declaration, and input.py keeps both the surface module and master's thermochem.fpp. check_amd_species_array_sizes now points at ${NUM_SPECIES}$, since AMD_NUM_SPECIES_MAX no longer exists.
sbryngelson
added a commit
to sbryngelson/MFC
that referenced
this pull request
Oct 1, 2026
Conflicts with MFlowCode#1918 in m_chemistry and the HLL/HLLC/LF Riemann solvers: kept this branch's per-solver case-optimization guards and applied MFlowCode#1918's change inside them -- species locals are declared once at ${NUM_SPECIES}$ instead of an AMD_NUM_SPECIES_MAX / num_species pair.
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
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
Follow-up to #1915 and #1852. Generic (non-case-optimized) amdflang builds declared species arrays with
AMD_NUM_SPECIES_MAX = 60andsys_sizearrays withAMD_SYS_SIZE_MAX = 70: 26 uses in 11 files, each an#:if not MFC_CASE_OPTIMIZATION and USING_AMDbranch. Now that MFC generatesm_thermochem, the species count is known at build time.build/include/<target>/thermochem.fppnext tocase.fpp:#:set NUM_SPECIES = <m_thermochem's num_species>and#:set CHEMISTRY = <bool>.dimension(${NUM_SPECIES}$)on every backend. Every speciesUSING_AMDbranch collapses to one declaration, and the per-thread arrays carry the mechanism's exact count (10 for h2o2, 11 for the Add heterogeneous reacting surface boundary conditions #1821 carbon mechanism) instead of 60. Raise the amdflang species bound so mechanisms above ten species build #1852-style "raise the bound" changes are no longer needed.AMD_SYS_SIZE_MAX = 10 + NUM_SPECIESfor chemistry builds (chemistry pinsnum_fluidsto 1, leaving at most 10 flow variables), so HLLC star states and CBCLvectors shrink from 70 to 20 for h2o2. Non-chemistry builds keep 70, so no non-chemistry case loses headroom. Thesys_sizecheck stays; the species-count check goes with the bound.#:includein the including file's directory before-Ipaths, so the defaultthermochem.fpp(for Fypp runs outside the toolchain) lives insrc/common/include/defaults/, whichcmake/Fypp.cmakesearches last.A literal extent rather than
dimension(num_species): my first attempt used thenum_speciesparameter directly and failedC4EB58A8on amdflang GPU (large tolerance mismatch). That run overlapped another build in the same checkout, so I do not claim it proves an amdflang bug, but the literal is what the former fallback used, is identical on every compiler, and passes cleanly.Performance
MI250X (one GCD), amdflang 23.2.1, generic
--gpu mp --no-mpibuild,examples/2D_premixed_flame_vortex(1024x512, h2o2, HLLC, reactions + diffusion), 500 steps, interleaved runs:fef3c276Every run of this PR is faster than every master run. All 17 output fields are bit-identical to master at step 500.
Testing
OMP_TARGET_OFFLOAD=MANDATORY):./mfc.sh test --gpu mp --only Chemistry- 16 passed, 0 failed. This is the configuration the guards applied to.--only Chemistry- 16 passed, 0 failed.NUM_SPECIES=10,AMD_SYS_SIZE_MAX=20for an h2o2 chemistry build) and that the default is used only when no generated file exists../mfc.sh precheckpasses.Interaction with #1821
#1821 adds new
${AMD_NUM_SPECIES_MAX}$declarations inm_ibm.fpp. Whichever of the two merges second needs them switched to${NUM_SPECIES}$; that is mechanical and I will do it.Prepared with Claude Code.
Contribution Policy
We do not accept pull requests generated primarily by AI without genuine understanding or real-world usage context.
All contributions are expected to demonstrate:
If these expectations are not met, we would prefer to implement the changes ourselves rather than spend time reviewing low-effort submissions.
Acknowledgement
PR template credit: junegunn