Repository navigation
[SC26 SCC] NTU Team Supernova's GPU optimizations to MFC - #2093
aliencaocao wants to merge 10 commits into
Conversation
|
Claude Code Review Head SHA: 0221014 Files changed:
Findings:
Checked and found consistent:
Coverage note: about 35k diff lines were not reviewed hunk-by-hunk. Only |
GPU performance optimizations for the simulation solver, mostly for the NVIDIA OpenACC path, together with fixes to existing numerical bugs. Most optimizations are opt-in via compiler arguments; the remainder are always-on (guarded by the relevant compiler/GPU where necessary) because guarding them would duplicate too much code. The work is directed by three findings: with case optimization MFC is largely not memory-capacity constrained; on modern GPUs the solver is memory-bandwidth bound through excessive device round-trips; and communication is a bottleneck. The changes therefore trade extra memory for on-device data residency and fewer H2D/D2H trips, maximize on-device cache/register/shared-memory reuse, and improve communication on both GPU-direct MPI and a new NVSHMEM transport.
|
The Intel compiler failures is due to a regression in ifx 2026.2. I tested locally with 2026.1 and it compiles fine. Seems to be happening on unmodified ifx segfaults on m_variables_conversion.mod. Flag-independent (segfault at -O0, no -ipo/-march). I will fix all the lint/format tests |
…-CMake renders MFC_FUSION_STAGING_DEFAULT is injected into the generated case.fpp by the toolchain only (toolchain/mfc/run/input.py). Documentation and other plain-CMake renders resolve case.fpp from src/common/include/defaults, which does not define it, so fypp aborted with a NameError and the documentation build failed. Keep the toolchain define first (same value) and fall back to the literal, mirroring the sibling constants below and toolchain/mfc/tuning.py. Co-Authored-By: Claude Code <noreply@anthropic.com>
…ng the cell list 269c8b8f dropped the GPU_UPDATE(host=[lbk_s]) at the top of s_build_cell_list, so the counting sort ran on unsynced positions and the smearing kernel dereferenced the resulting garbage cell-list indices -- a deterministic illegal address at the first step of every smooth-type-1 Lagrange case under --lagrangian-device-cell-list. Restore the line as it stood at 306d32f4. Verified: the 27-case Lagrange family passes with all 20 opt-in flags. Co-Authored-By: Claude Code <noreply@anthropic.com>
The classic sweep's populate updates the permuted sweep bounds (is1/is2/is3 and the direction tables) with !$acc update device on the captured queue inside the region. The PGI acc runtime's update path queries a per-queue event before each copy; once enough preceding launches have recorded that event on the capturing stream, the query returns CUDA_ERROR_CAPTURED_EVENT, which invalidates the capture, and the next copy fails with CUDA_ERROR_STREAM_CAPTURE_INVALIDATED (901) at the first capture attempt after the warmups. This took out every classic-path case that reached capture: the hypoelasticity-HLLC, IBM-example and chemistry suites. Only the fused path, which stages its own bounds as launch arguments and issues no in-region device updates, is captured. Co-Authored-By: Claude Code <noreply@anthropic.com>
The transport loops took the direction offsets as a firstprivate array, which the acc runtime uploads with a host-to-device copy at every launch. Inside a captured region that copy rides the capturing stream, and the runtime's queue-event bookkeeping invalidates the capture (error 901 at the first captured launch; the chemistry diffusion runs three directions per stage, so its capture starts during the first step). The fused DIFF_TRANSPORT branch already used scalar offsets; hoist that pattern to all transport models and drop the now-unused module-level offsets array. Co-Authored-By: Claude Code <noreply@anthropic.com>
nvfortran heap-allocates the fused WENO column's per-equation scratch in device code (about 450 B per equation per gang, malloc/free pairs every launch). The CUDA default 8 MiB device heap then exhausts once enough gangs are resident -- 3D grids reach 13-24 MiB -- and the first scratch access after a failed malloc surfaces as a warp MMU fault inside the fused kernels, at an instruction that varies run to run. Domains whose gangs stay under the limit (1D/2D) pass, which is why only 3D cases faulted. Raise cudaLimitMallocHeapSize to 256 MiB after the device is selected; the heap still allocates on demand, the limit only caps its growth. With the heap raised, 3D -> Bubbles -> QBMM -> bubble_model=3 matches its golden file. Co-Authored-By: Claude Code <noreply@anthropic.com>
MFC_REGISTER_CAPS validated every module against src/simulation only, but m_eos and m_variables_conversion live in src/common while the simulation target still compiles them into fypp/simulation, so naming either one aborted the build with 'unknown solver module' -- exactly the two callee translation units a capped m_riemann_solver_hllc needs capped alongside (nvlink refuses a capped kernel calling an uncapped routine needing more). Co-Authored-By: Claude Code <noreply@anthropic.com>
…ticity The ADC (HLL->HLLC) path declares seven dimension(sys_size) work arrays and emits a second, hypoelastic-specialized copy of the 5-equation kernel in every build. Neither executes on non-hypoelastic cases, but the variable-extent automatics still force a local-memory frame on the pure-fluid kernel: the 200^3 performance case's HLLC kernel drops from 400 to 144 B of stack and 1.90 to 1.68 ms, matching what the 168-register cap bought. Co-Authored-By: Claude Code <noreply@anthropic.com>
register_caps(case) refuses the Riemann-solver caps on chemistry cases (s_compute_chemistry_average_state sits at 254 registers, and viscous chemistry adds the generated mixture viscosity at 228; nvlink refuses the link) and the m_variables_conversion cap on the 6-equation model (the isentrope spill under a cap is unmeasured). Setting caps on a case whose Riemann solve the fused lane carries prints a harmless-does-not-bind warning instead of selecting anything. Co-Authored-By: Claude Code <noreply@anthropic.com>
The grid-cell patch loop writes ib_markers with the same atomic max the ib-parallelism path uses, so overlapping patches pick the highest encoded patch id deterministically instead of the last loop index. q_T_sf's deallocation takes the allocation's .not. igr guard (an igr build with heat conduction deallocated an unallocated workspace), chemistry refuses to run without it, and the classic Riemann flux scratch is declared stp rather than wp so a mixed-precision build stores it at the storage precision. Co-Authored-By: Claude Code <noreply@anthropic.com>
Lines of Code
|
|
@aliencaocao While there are some interesting ideas in this PR, these changes will not make it into MFC by the October 26th deadline for SCC changes. This PR also exceeds the +/- 5000 line guideline by ~5x. If you'd like to make any of these contributions, you can submit them one at a time, but they won't be approved and merged by October 26th. |
|
@wilfonba totally understand and was not expecting anything to be merged by 26 Oct. I think this serves more as a discussion starter to what kind of change is the maintainer team more open to, and which are the things like "ok it improve performance but we are still not going to accept". I can try to isolate some smaller changes meanwhile to see if they can still make it. |
|
@aliencaocao I think you'd be better off spending your time on preparations that will actually help during the competition. This code has more problems than just its sheer quantity. The source is littered with |
|
@aliencaocao I would strongly encourage you to read up on the intended application of MFC and philosophies on contributing to open-source codes, especially open-source scientific code. There are two flaws with this methodology of development that run counter to the design principles of MFC.
The respectful way to go about this is to break this down into chunks that are able to be reviewed by a human. The 5k limit that @wilfonba put on the contributions for this competition were quite generous, and generally I would expect all by the largest of changes to fit under 1k lines of additions. I also think that he is optimistic in saying it would not be merged by October 26th. This PR, as stands, has no chance of ever being merged for the reasons stated above. |
|
Thank you both for your suggestions. I think there is some misunderstanding about how I approached this and I would like to clarify.
It was a missed out line that i accidently deleted when moving from a internal fork branch to the public in a follow-up commit. It passed all tests in the initial PR state. I have already fix it and I was waiting my local full-CI run to pass again before I push the fix. In addition, I have ran the full CPU + GPU test suite on both NVIDIA and AMD GPUs, and all of them passed.
I agree it is very messy, they were there to keep the original behaviour so it is easier to test individual changes on/off for review puporse. These can all be removed if the changes become alwyas-on code path. More than half of the apprant LOC you see are from preserving existing code paths and indentation changes enforced by the formatter.
That is a limitation of current compilers and inlining, which causes performance regressions as new features are being added for modularization (like the HLLC kernel I reported and spent a day investigating). I cannot come up with a more elegant solution. Hope this gets solved in future. @danieljvickers ,
I agree that the strong scaling efficiency drops but I would not say I am optimizing for any particular cluster, large or small scale. They dropped simply due to much faster per-device compute time, leading to previoulsy hidden communication cost to become larger part of the runtime, which is much harder to "hide" due to physical limits such as communication latency and bandwidth. End to end runtime is still be much faster as shown. From my knowledge and testing, none of my changes will make communication alone slower at a larger scale. This can be proven by the roughly on-par weak scaling I did which shows that it maintained close to perfect when maintaining per-device work to be constant.
This is only true for strong scaling efficiency, and again that is from the increase in communication time as the proportion of total runtime. If you see weak scaling, I would argue that the PR does not lose to Without access to a larger cluster, I am unable to claim or test anything on scaling efficiency of my changes. That is why I focused more on compute-bound part of the code, which currently, as pointed out by the MFC 5.0 paper, reaches only 5% of roofline on H200. I saw the gap and I genuinely wanted to help improve it.
I agree, and I apologise sincerely if this made me seem disrespectful to the maintainers. I did not intend it this way, and if by any means I did, please accept my apology. I undetstand the importance of scientific tracebility. But please allow me to give more context on this. These code changes has been in work for more than 6 months even with help of AI, it is not some AI slop that I generated in a week and just let it out. Every change was prompted by human analyzing performance traces and profiles, and tracing SASS back line-by-line to fortran to identify the slow part. There is a reason I was able to spot existing bugs while working on these (if you would see the bug reports I raised and their individual fixes, most of them have been proven to be correct by one of the maintainers), because I stress a lot on maintaining scientific correctness when doing all these and i checked all the numerical results where they disagree. I (not AI) have written a detailed PR description which I attached in the PR as file to explain every change and show performance results where applicable. These were geniue human hours put into this, and it may just not be presented correctly at the correct time. I agree that a single huge PR is hard to handle but like I explained in the description, it is intended to manage the CI runners. I will stop pursuing this PR completely. I apologise if any of these wasted your time. I hope at the very least the ideas and bottlenecks identified in the PR can help to inspire some directions for improvement. |
3 participants
[SC26 SCC] NTU Team Supernova's GPU optimizations to MFC
For the full version of the PR description, please refer the full version below, as they are too long for Github to accept.
PR.md
This PR adds GPU performance optimizations for the simulation solver and communication optimizations, mostly for NVIDIA OpenACC path. It also fixes some existing numerical bugs. Most of the optimizations are guarded via opt-in compiler argument, except for some that will cause too much duplicate code when guarded. For those, they are always-on and guarded by the relevant compiler/GPU where necessary.
If I should be adding the options as case.py arguments, please let me know too, I know that is the current pattern, but I think case.py should focous on defining case-specific options and not general performance opt-ins. This is my own decision, and I am happy to change the approach if the maintainers prefer otherwise.
This is a large PR and if the maintainers prefer to split them into smaller PRs, I can do that too, but the goal was to not flood the CI runners so I merged them into 1 single PR.
If the maintainers are satisfied with some of the lower-risk optimizations and would like to make them always-on as default, I can also remove their respective opt-in toggles by replacing the existing implementation. This reduces code duplication. A significant part of the changed LOCs are from preserving existing code paths.
This PR assumes existing bug-fix PRs by me will be merged, and I will handle any conflicts arising from other team's contributions where necessary.
In general, the direction that I am working towards to is based on the following findings:
Thus, you will find many optimizations that trades some extra memory usage for data residency/reduced H2D/D2H trips, and maximizing on-device cache/register/shared memory reuse.
Unfortunately I do not have access to any larger cluster like Frontier to test scalability of my changes, so I want to focus more on improving compute-bound parts, and I do not want to claim anything in scalability for communication changes beyond what I have tested on.
Benchmark environment
All performance figures in this description were measured on 1-8 NVIDIA H200 NVL (141 GB) with 4-way NVLink bridge at 600W TDP · NVHPC 26.9 · CUDA 13.4 · NVSHMEM 3.8.0 · OpenACC · FP64 unless otherwise specified.
For fair comparison, all runs enabled GPU-direct MPI exchange
(
rdma_mpi) to achieve optimal performance on NVLinked NVIDIA GPUs. This is by default off in the current code and this PR does not change that default.Primary Results
Measured on https://github.com/MFlowCode/MFC/blob/master/examples/3D_performance_test/case.py 200³ performance test, 300 steps, one rank per GPU. The results are measured with kernel tuning configuration for our specific hardware (H200 NVL PCIe). The exact tuning file may not transfer to every case even on same hardware, and thus is only worth once a large run is about to start.
The tables give the median grind time of the runs at each point as reported by MFC. As reported in #2088, current
mastercode has a significiant regression in this test case (and many others). For our hardware we expected pre-regression timing to be around 0.34ns. The regression has been identified and fixed in this PR, but the numbers below is unaffected by it because one of the optimizations prevented the regression by construction.This PR includes an improved version of #1628. To ensure fairness and isolate geniue contributions, the number and scaling study below EXCLUDES this optimization. It also varies greatly with each case so it is unfair to base it all on the 3D performance test case. In Section 12, I separately report how the optimization further improves the performance.
8-GPU runs were 2 groups of 4xH200 NVL within a single node running dual AMD EPYC 9755 CPU.
Step time in ms/step
The two transports agree on the grind within 2% at 2, 4 and 8 GPUs, however due to removal of MPI host-stall outside of the grind time region, NVSHMEM runs 8%, 7% and 25% faster at 2, 4 and 8 GPUs (§6), judging from step time. This is meaningful to total runtime which the current grind time fail to account for. More info on NVSHMEM below.
Strong scaling
The original code scales 1.89×, 3.44× and 5.85× at 2, 4 and 8 GPUs. This PR
scales 1.79×, 3.06× and 4.40× on NVSHMEM, and 1.80×, 3.11× and 4.38× on
GPU-direct MPI. The speedup over the original code falls from 8.0× at one GPU
to 6.0× at 8 GPUs.
In host step time the original scales 1.89×, 3.45× and 5.84×. This PR scales 1.52×, 2.60× and 3.41× on NVSHMEM, and 1.40×, 2.44× and 2.74× on GPU-direct MPI. The step time contains the host side of the exchange, which the device-timed grind excludes, so it scales less than the grind.
Weak scaling
The work per GPU stays at 100³ cells. The grids are 100³, 200×100×100, 200×200×100 and
200³ on 1, 2, 4 and 8 GPUs.
The original code holds perfect weak efficiency at our scale. This PR runs every point 6.0× to 6.9× faster in grind. Its weak efficiency is 1.12, 1.10 and 0.98 at 2, 4 and 8 GPUs on NVSHMEM. On GPU-direct MPI it is 1.13, 1.13 and 0.97.
The 8-GPU runs place the ranks in a 1×2×4 grid which is most optimized for our specific topology. The 2-GPU and
4-GPU runs and the original code use the built-in most-cubic Cartesian cuts.
The performance-test case applies characteristic boundary conditions (CBC) on all six faces. At 1 GPU the single rank need to do the CBC staging on all 6 physical faces. On multi rank, the domain is split and the cut faces become rank interfaces, which exchange halos but run no CBC staging. The slowest rank's physical-face count falls 6 → 5 (2 GPUs) → 4 (4 GPUs) → 3–4 (8, where x is unsplit). With less boundary work per rank, the weak-scaling efficiency become higher than 100% at 2 and 4.
masterstays flat at 1.00 because its per-rank time is ~6× larger, so the same boundary saving is ~1–2% and invisible.Weak scaling at 8 GPU drops sharply partly due to the additional cross-NUMA traffic instead of NVLink, since we are not using a full-mesh node, so communication become much more bottlenecked. On H200 SXM I do not expect such a sharp drop.
Step time in ms/step
In host step time the original stays flat at 14.7 ms. This PR goes from 1.89ms at one GPU to 2.53 ms at eight on NVSHMEM, and to 3.15 ms on GPU-direct MPI. The grind-based weak efficiency at 8 GPUs is 0.98 on NVSHMEM, so the rise in step time is host-side work outside the device-timed region.
New Build options
Most of the optimizations in this PR are default-off and can be enabled by these new build options. Each option is a
./mfc.sh buildflag and a CMake option of the same name. For example,--fused-rhssetsMFC_FUSED_RHS=ON, and--no-fused-rhssets it toOFF. Every option is offby default. The toolchain (
toolchain/mfc/args.py) and CMake check the requirements below andstop with a message when any incompatible options are used, since some depends on others. Some options automatically enable the other dependent ones, unless the user explicitly passed --no-*, in which case they conflict and it will error with a custom error message.
--fused-rhs--case-optimization, OpenACC GPU build with NVHPC--hllc-register-tuning--case-optimization, OpenACC GPU build with NVHPC--cbc-no-host-sync--cbc-parallel-scatter--cbc-no-host-sync--async-queue--no-unified,--accurate-grind-time--accurate-grind-time--cuda-graphs--no-unified,--async-queueand therefore--accurate-grind-time--halo-single-round--mpi--halo-index-cache--halo-single-round--nvshmem--mpi, OpenACC GPU build with NVHPC,--no-unified, the NVSHMEM library--weno-direct-loads--case-optimization, OpenACC GPU build with NVHPC,--fused-rhsor--hllc-register-tuning--muscl-direct-loads--direct-riemann-flux--case-optimization, OpenACC GPU build with NVHPC--persistent-bc--halo-single-round.--fused-bubble-source--qbmm-device-inversion--case-optimization--igr-tuned-sweeps--case-optimization, otherwise it will be partially enabled.--igr-shallow-halo--lagrangian-device-cell-list--active-boxMFC_ACTIVE_BOXturns it on at run time.--fused-rhs, checked whenMFC_ACTIVE_BOXis setNew Environment variables
Run time
The binaries read these variables at start-up. An unset variable gives the behavior of the
original code.
MFC_ACTIVE_BOX<steps>[,<growth>[,<boxes>]]--active-box --fused-rhsMFC_ACTIVE_BOX_FLOOR--active-boxMFC_ACTIVE_BOX_INNERk≥ 1ksteps. It needs the single-box window.--active-boxMFC_HALO_OVERLAP1--fused-rhs --halo-single-round --async-queueMFC_HALO_EPOCH_DEVICE1--nvshmem --halo-single-roundMFC_HALO_26_TRIM1--nvshmem --halo-single-round --active-boxMFC_IGR_PINGPONG1MFC_SFC_WEIGHTSMFC_DECOMP_PROCESS_GRID<px>,<py>,<pz>parallel_io = T.MFC_DECOMP_AXIS_ORDER1,2,3parallel_io = T.1,2,3MFC_DECOMP_X_OFFSETS,MFC_DECOMP_Y_OFFSETS,MFC_DECOMP_Z_OFFSETS0,<cut>,...,<size>CUDA_VISIBLE_DEVICES,ROCR_VISIBLE_DEVICES,HIP_VISIBLE_DEVICES--accurate-grind-timeNVSHMEM_REMOTE_TRANSPORT,NVSHMEM_IB_ENABLE_IBGDA1--nvshmemBuild time
The toolchain reads these variables when it generates and builds a case.
MFC_TUNING_FILE<path>valueskey.--fused-rhsor--fused-bubble-sourceMFC_FUSION_*--fused-rhsMFC_BUBBLE_VECTOR_LENGTH--fused-bubble-sourceMFC_STATE_ALIGNMENT_BYTES--fused-rhsMFC_REGISTER_CAPS<module>=<cap>[,<module>=<cap>...]src/simulation. The cap is an integer from 1 to 255.--gpu accwith NVHPCNVSHMEM_ROOT<path>--nvshmemThe source gives the default of every constant. A value that is not set keeps the default.
Tests
This PR adds 1 golden
AE748A63to catch #2048. Assuming that the numerical bug PRs by me are merged.CPU GCC 16.2: all golden tests pass on AMD EPYC 9755 with all optimizations off (except for those default-on ones)
NVHPC + OpenACC (NVHPC 26.9): all golden tests pass on H200 with all optimizations ON
NVHPC 26.9 + OpenMP: the simulation target compiles with all optimizations off. Golden cases not ran.
AMD + OpenMP (AMDFlang 24.0.0git from therock-afar-24.3.0, MI210): all 727 golden tests pass with BOTH all optimizations off and all on. All 17 convergence cases pass with all off and on.
I do not have access to any CCE systems so am unable to test any of those, but I have tried my best to ensure my changes are contained and do not expect any to break CCE
Design notes
files, and post-processing pipelines stay compatible. The optimization flags
compose through compile-time configuration.
layer and module conventions of the source. The fused kernel uses fypp templates over
direction and physics, like the existing preprocessor usage.
Fixes #2088 #2048
For HLLC kernel, to reach the claimed 1.38ms performance, register capping is needed:
MFC_REGISTER_CAPS=m_riemann_solver_hllc=168,m_riemann_solver_hll=168,m_riemann_state=168,m_hb_function=168,m_variables_conversion=168,m_eos=168Without, the kernel runs 1.70ms.
Addresses the hang observed in #1927
Related:
#2090 where I believe my implementation is better for the cases where it covers, and #2090 is better for other cases.
#2092 came up with similar idea independently but I chose to go the more extreme route of optimizing away the loads using fused kernels. also optimize away "
mp_wenoandint_comp > 0" mentioned in the PR.#1628 where I improved upon by supporting multi-rank natively.
Contribution Policy
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
This PR is prepared by Billy Cao from Nanyang Technological University SCC team: Supernova for SC26 Student Cluster Competition. It unintentionally overlapped with ideas that existing PRs from other teams have raised, and I have fully acknowledged them in these cases. It also took inspiration from existing PRs by the MFC team, which I have also fully credited where due. For each of such cases, I have explained the differences/improvements on top of existing PRs.