Repository navigation
Fix host and device memory leaks in module setup and teardown - #2155
Open
sbryngelson wants to merge 1 commit into
Open
sbryngelson wants to merge 1 commit into
sbryngelson wants to merge 1 commit into
Conversation
- m_surface_tension: c_divs has num_dims + 1 fields but finalize freed only num_dims of them, leaking the gradient-magnitude field. - m_cbc: pi_coef_y/z are allocated when weno_order > 1 or muscl_order > 1 but were freed only for weno_order > 1, leaking them in MUSCL runs with characteristic BCs. Use the x-direction condition. - m_global_parameters (pre/sim/post): MPI_IO_DATA%var(i)%sf and MPI_IO_IB_DATA%var%sf were allocated and then immediately nullified, leaking every buffer. The pointers are default-initialized to null and are associated later by s_initialize_mpi_data, so drop the allocate loops. Free MPI_IO_DATA%var/view unconditionally in finalize to match the unconditional allocation (they leaked when parallel_io was off). - m_global_parameters (sim): neighbor_ranks was freed after the n == 0 and p == 0 early returns, so 1D/2D runs leaked it. Free it first. - m_data_output (sim): the file_per_process path called s_initialize_mpi_data a second time (identical pointers, leaked MPI derived types) and created the restart directory twice. Down-sampled output writes q_cons_temp_ds directly, and down_sample requires IGR, which excludes Euler bubbles, so the second call had no effect. No change to results: the debug CPU test suite passes unchanged, and 2-rank NVHPC MPI runs with parallel_io (2D IBM with a shared file and with file_per_process; 3D IGR file_per_process with and without down_sample) give byte-identical restart data and identical silo output (h5diff) against master.
|
Claude Code Review Head SHA: b66ed8c Files changed:
Findings: |
Lines of Code
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2155 +/- ##
==========================================
- Coverage 61.90% 61.89% -0.01%
==========================================
Files 86 86
Lines 22948 22934 -14
Branches 3386 3380 -6
==========================================
- Hits 14205 14196 -9
Misses 6229 6229
+ Partials 2514 2509 -5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This branch has not been deployed
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.
What
only num_dims of them, leaking the gradient-magnitude field.
muscl_order > 1 but were freed only for weno_order > 1, leaking them
in MUSCL runs with characteristic BCs. Use the x-direction condition.
MPI_IO_IB_DATA%var%sf were allocated and then immediately nullified,
leaking every buffer. The pointers are default-initialized to null and
are associated later by s_initialize_mpi_data, so drop the allocate
loops. Free MPI_IO_DATA%var/view unconditionally in finalize to match
the unconditional allocation (they leaked when parallel_io was off).
and p == 0 early returns, so 1D/2D runs leaked it. Free it first.
s_initialize_mpi_data a second time (identical pointers, leaked MPI
derived types) and created the restart directory twice. Down-sampled
output writes q_cons_temp_ds directly, and down_sample requires IGR,
which excludes Euler bubbles, so the second call had no effect.
No change to results: the debug CPU test suite passes unchanged, and
2-rank NVHPC MPI runs with parallel_io (2D IBM with a shared file and
with file_per_process; 3D IGR file_per_process with and without
down_sample) give byte-identical restart data and identical silo output
(h5diff) against master.
Verification
Testing. Built with GPU offload (OpenMP, AMD) and ran the full test suite with this fix and the species-bounds change (#2154) on one test branch: 731 tests in 4 shards, 0 failures (MI300A and MI250X).
./mfc.sh precheckpasses.Prepared with Claude Code (AI-assisted); I reviewed the change and ran the tests.