fix: free tau_Re_vf in all viscous runs, not only cyl_coord

[sbryngelson]

Summary

Fixes #1366.

In s_finalize_rhs_module, tau_Re_vf is allocated inside if (viscous) but was only deallocated inside if (cyl_coord). Any Cartesian viscous run (viscous = .true., cyl_coord = .false.) leaked all tau_Re_vf host and GPU allocations.

The fix removes the erroneous if (cyl_coord) guard — the deallocation is already inside the if (viscous) block, so no additional condition is needed.

Change

src/simulation/m_rhs.fpp: drop the if (cyl_coord) then ... end if wrapper around the three tau_re_vf @:DEALLOCATE calls.

Test plan

• Existing viscous regression tests pass
• Existing cylindrical-coordinate viscous tests pass (deallocation still fires via the outer if (viscous) guard)

[github-actions]

Claude Code Review

Head SHA: dcb8a72

Files changed:

• 1
• src/simulation/m_rhs.fpp

Findings

Memory management: @:DEALLOCATE guard removed without confirming @:ALLOCATE is unconditional

src/simulation/m_rhs.fpp — deallocation hunk

The if (cyl_coord) guard is removed from three @:DEALLOCATE calls for tau_re_vf:

-                if (cyl_coord) then
-                    do i = 1, num_dims
-                        @:DEALLOCATE(tau_re_vf(eqn_idx%cont%end + i)%sf)
-                    end do
-                    @:DEALLOCATE(tau_re_vf(eqn_idx%E)%sf)
-                    @:DEALLOCATE(tau_re_vf)
-                end if
+                do i = 1, num_dims
+                    @:DEALLOCATE(tau_re_vf(eqn_idx%cont%end + i)%sf)
+                end do
+                @:DEALLOCATE(tau_re_vf(eqn_idx%E)%sf)
+                @:DEALLOCATE(tau_re_vf)

Per CLAUDE.md: "Conditional allocation MUST have conditional deallocation."

The matching @:ALLOCATE calls for tau_re_vf and its .sf components are not shown in this diff. If those allocations are still guarded by if (cyl_coord), then attempting to deallocate these arrays when cyl_coord = .false. will cause a Fortran runtime error (deallocation of an unallocated variable). The change is safe only if the allocation side is also unconditional. Please verify that the @:ALLOCATE calls for tau_re_vf and tau_re_vf(...)%sf are unconditional (or have been made unconditional as part of this fix).

[sbryngelson]

The fix is correct. The allocation of tau_Re_vf at line 333 of m_rhs.fpp is guarded only by if (viscous) — there is no cyl_coord guard anywhere near the allocation:

if (viscous) then
    @:ALLOCATE(tau_Re_vf(1:sys_size))
    do i = 1, num_dims
        @:ALLOCATE(tau_Re_vf(eqn_idx%cont%end + i)%sf(...))
    end do
    @:ALLOCATE(tau_Re_vf(eqn_idx%E)%sf(...))
    ...
end if

The old code had the deallocation incorrectly nested inside if (cyl_coord), which had no corresponding guard on the allocation — that asymmetry was the original bug causing a memory leak in Cartesian (cyl_coord = .false.) viscous runs. The fix removes the spurious cyl_coord guard from the deallocation so both allocation and deallocation are symmetric: they both execute whenever viscous = .true., regardless of cyl_coord.

[qodo-code-review]

ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e9fbcf57-7fc2-4174-9d4b-b9f876f561c3

📥 Commits

Reviewing files that changed from the base of the PR and between d513442 and d339288.

📒 Files selected for processing (1)

• src/simulation/m_rhs.fpp

---

📝 Walkthrough

Walkthrough

This change corrects the finalization logic in s_finalize_rhs_module for the viscous stress temporary field tau_Re_vf. The prior deallocation was conditional, only executing when cyl_coord was true. The updated logic deallocates all components unconditionally: the stress field components across spatial dimensions, the energy component, and the parent container. This ensures consistent memory cleanup in all coordinate systems and aligns the deallocation pattern with the allocation logic in s_initialize_rhs_module.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main fix: correcting tau_Re_vf deallocation to apply to all viscous runs instead of only cylindrical coordinate runs.
Description check	✅ Passed	The description comprehensively explains the memory leak (issue #1366), the root cause (asymmetric allocation/deallocation guards), the fix (removal of cyl_coord guard), and provides a clear test plan.
Linked Issues check	✅ Passed	The PR fully addresses #1366 by fixing the tau_Re_vf memory leak through removal of the erroneous cyl_coord guard, ensuring symmetric allocation/deallocation pairing [#1366].
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the memory leak in tau_Re_vf deallocation within m_rhs.fpp, with no unrelated modifications or scope creep.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sbryngelson]

Automated Code Review

Summary: No issues found. ✅

Verified:

• All three tau_Re_vf allocation levels are now symmetrically paired with their deallocations after removing the if (cyl_coord) guard
• GPU deallocation is fully covered — @:DEALLOCATE macro always emits GPU_EXIT_DATA(delete=...) before the Fortran deallocate
• No double-free risk: tau_Re_vf is allocated in exactly one place (inside if (viscous)) and deallocated in one matching block
• The igr path is consistent: both allocation (if (.not. igr)) and deallocation (if (.not. igr)) carry the same guard — igr=true runs unaffected

Minimal, correct fix. Safe to merge.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.90%. Comparing base (c99ac2d) to head (e1df749).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1398      +/-   ##
==========================================
+ Coverage   64.89%   64.90%   +0.01%     
==========================================
  Files          72       72              
  Lines       18877    18873       -4     
  Branches     1571     1570       -1     
==========================================
  Hits        12250    12250              
+ Misses       5651     5648       -3     
+ Partials      976      975       -1     

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