fix: add missing @:DEALLOCATE in simulation modules

[SVS87]

Description

Part of #1459

Add matching @:DEALLOCATE calls in:

• m_derived_variables
• m_global_parameters
• m_hyperelastic
• m_rhs
• m_riemann_solvers
• m_time_steppers

Remaining files not yet addressed:

• src/common/m_boundary_common.fpp, m_helper.fpp, m_model.fpp, m_mpi_common.fpp, m_variables_conversion.fpp
• src/post_process/m_start_up.fpp
• src/simulation/m_acoustic_src.fpp, m_bubbles_EE.fpp, m_ibm.fpp (pending PR Local Aware IBM #1378), m_qbmm.fpp

Type of change

• Bug fix
• New feature
• Refactor
• Documentation
• Other: describe

Testing

Ran ./mfc.sh test -j 4 locally on Ubuntu/WSL with GNU compiler and MPI.
578 tests passed, 0 failed. Spelling and lint checks passed.

Checklist

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

See the developer guide for full coding standards.

GPU changes (expand if you modified src/simulation/)

• GPU results match CPU results
• Tested on NVIDIA GPU or AMD GPU

AI code reviews

Reviews are not triggered automatically. To request a review, comment on the PR:

• @coderabbitai review — incremental review (new changes only)
• @coderabbitai full review — full review from scratch
• /review — Qodo review
• /improve — Qodo code suggestions
• @claude full review — Claude full review (also triggers on PR open/reopen/ready)
• Add label claude-full-review — Claude full review via label

[codecov]

Codecov Report

❌ Patch coverage is 25.92593% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.27%. Comparing base (016c763) to head (4f9de6c).
⚠️ Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
src/simulation/m_derived_variables.fpp	0.00%	7 Missing and 2 partials ⚠️
src/simulation/m_rhs.fpp	36.36%	5 Missing and 2 partials ⚠️
src/simulation/m_time_steppers.fpp	40.00%	1 Missing and 2 partials ⚠️
src/simulation/m_hyperelastic.fpp	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1476      +/-   ##
==========================================
- Coverage   61.31%   61.27%   -0.04%     
==========================================
  Files          72       72              
  Lines       19771    19793      +22     
  Branches     2852     2858       +6     
==========================================
+ Hits        12123    12129       +6     
- Misses       5699     5709      +10     
- Partials     1949     1955       +6     

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

[sbryngelson]

Code Review

Reviewed the diff by checking each newly-added @:DEALLOCATE against its allocation site (guard conditions, allocation rank, and whether it was allocated with @:ALLOCATE).

Overview

This PR (part of #1459) replaces a few bare deallocate calls with the @:DEALLOCATE macro and adds matching deallocations for arrays that were previously leaked at finalization across six simulation modules. The motivation is sound: @:DEALLOCATE pairs with @:ALLOCATE so that device (OpenACC/OpenMP) mappings are released alongside the host array, whereas a bare deallocate leaves the device copy mapped.

Correctness — verified symmetric ✅

For these, the new deallocation matches the allocation guard and structure exactly:

• m_derived_variables.fpp — accel_mag, x_accel, fd_coeff_{x,y,z} are all allocated under if (probe_wrt) (with the n>0/p>0 nesting). Moving the fd_coeff deallocation inside the probe_wrt block and switching from if (allocated(...)) to the n>0/p>0 guards is correct and behavior-preserving, and it now also releases the device copies (these are in GPU_DECLARE(create=...)). Good cleanup.
• m_global_parameters.fpp — ptil: alloc and dealloc both under if (bubbles_euler). ✅
• m_hyperelastic.fpp — Gs_hyper(1:num_fluids): allocated unconditionally in init, deallocated unconditionally in finalize. ✅
• m_riemann_solvers.fpp — Gs_rs unconditional both sides; Res_gs under if (viscous) both sides. ✅
• m_time_steppers.fpp — checked each: q_T_sf%sf (chemistry), pb_ts(1:2)/pb_ts(1,2)%sf/rhs_pb and the mv_ts set (unconditional — one of the three alloc branches always runs), max_dt (cfl_dt), bc_type (loop i=1,num_dims matches the n>0/p>0 allocation since num_dims tracks dimensionality), rk_coef (any(time_stepper==[1,2,3])). All symmetric. ✅
• m_rhs.fpp — alf_sum%sf (mpp_lim .and. bubbles_euler); the qbmm block (mom_3d(0:2,0:2,1:nb)%sf, mom_sp(1:nmomsp)%sf, then containers) matches the allocation exactly; blkmod1 (alt_soundspeed). The replacement of the explicit $:GPU_EXIT_DATA(delete='[alf_sum%sf]') + deallocate with @:DEALLOCATE(alf_sum%sf) is equivalent and cleaner. ✅

Issue — incomplete deallocation of qL_prim / qR_prim (m_rhs.fpp) ⚠️

This is the one spot I'd flag. The allocation (≈ lines 264–284) builds a nested structure:

@:ALLOCATE(qL_prim(1:num_dims))
do i = 1, num_dims
    @:ALLOCATE(qL_prim(i)%vf(1:sys_size))
    do l = eqn_idx%mom%beg, eqn_idx%mom%end
        @:ALLOCATE(qL_prim(i)%vf(l)%sf(...))   ! GPU create + @:ACC_SETUP_VFs
    end do
    @:ACC_SETUP_VFs(qL_prim(i), qR_prim(i))
end do

but the new finalization only does:

@:DEALLOCATE(qL_prim)
@:DEALLOCATE(qR_prim)

The inner %vf(l)%sf arrays were created on the device via @:ALLOCATE/@:ACC_SETUP_VFs. @:DEALLOCATE(qL_prim) only issues an exit-data delete for the top-level container, so those inner device mappings are not released — a device-memory leak. On the host side Fortran recursively frees the allocatable components, so there's no host leak, but the GPU side is left incomplete. This is exactly the case the PR is trying to fix elsewhere.

The flux_n/flux_src_n/flux_gsrc_n cleanup immediately above is the template to follow — it walks the structure and deallocates each %vf(l)%sf and %vf before the container. Suggest mirroring that, e.g.:

do i = 1, num_dims
    do l = eqn_idx%mom%beg, eqn_idx%mom%end
        @:DEALLOCATE(qL_prim(i)%vf(l)%sf)
        @:DEALLOCATE(qR_prim(i)%vf(l)%sf)
    end do
    @:DEALLOCATE(qL_prim(i)%vf, qR_prim(i)%vf)
end do
@:DEALLOCATE(qL_prim, qR_prim)

(Confirm whether any of those %sf are pointer-associated rather than owned before deallocating — flux_src_n above nullifys some entries, so the same care may apply here.)

Testing note

The reported "578 tests passed" on a GNU+MPI CPU build is good for confirming no host-side regression, but it won't exercise the device-side exit-data paths that are the whole point of switching to @:DEALLOCATE. Since these all occur at module finalization (program teardown), a leak won't surface as a test failure even on GPU. A GPU build/run (and ideally a memory check) would give more confidence that the device mappings are actually balanced.

Summary

Solid, well-scoped refactor; 5 of 6 files are clean and symmetric. The only substantive item is the incomplete qL_prim/qR_prim teardown in m_rhs.fpp, which leaves device sub-arrays mapped — worth completing to match the allocation and the adjacent flux_n pattern.