Severity: medium · Category: memory-ub
Locations: src/projection/incflo_apply_cc_projection.cpp:344-352, src/projection/incflo_apply_cc_projection.cpp:385, src/projection/incflo_apply_cc_projection.cpp:406-408, src/projection/incflo_apply_cc_projection.cpp:416, src/projection/incflo_apply_cc_projection.cpp:430-431
Based on commit 46de3367 (line numbers refer to that tree).
The defect
Vector<MultiFab*> cc_phi(finest_level+1);
Vector<MultiFab*> cc_gphi(finest_level+1);
for (int lev = 0; lev <= finest_level; ++lev )
{
cc_phi[lev] = new MultiFab(grids[lev], dmap[lev], 1, 1, MFInfo(), Factory(lev));
cc_gphi[lev] = new MultiFab(grids[lev], dmap[lev], AMREX_SPACEDIM, 0, MFInfo(), Factory(lev));(lines 344-349). There is no delete anywhere in the function (or the file: grep -n delete src/projection/*.cpp is empty), so two full-domain
MultiFabs (1 + AMREX_SPACEDIM components) leak on every call.
Prior-round issue #191 reported exactly this. PR #213 (25a2d140, "fix issues 171 176 181 188 189 191", closes #191) touched this
file, but its only hunk is the relocation of the U*-U^n subtraction for #181;
git diff 7307d872..HEAD -- src/projection/incflo_apply_cc_projection.cpp | grep -n "new MultiFab\|delete" returns nothing. The issue was closed
without the fix landing.
Why it matters
ApplyCCProjection runs once per predictor (and once per corrector with MOL) plus the initial projection and each initial iteration. With
incflo.use_cc_proj = 1 the resident memory grows by (1+AMREX_SPACEDIM) cell-centred fields per projection call; on GPU this is device
memory, so a long run eventually fails with an out-of-memory error in the Arena. The nodal path uses projector-owned storage and has no such leak.
How to reach it
Any input with incflo.use_cc_proj = 1 (amr.max_level = 0 is enforced for it). No shipped test sets it. DIM 2 or 3, non-EB (EB aborts at line 414 before
the leak matters). CPU or GPU; on GPU the leak is device memory.
Suggested fix
Hold the temporaries in Vector<MultiFab> and hand pointers to Hydro with GetVecOfPtrs, exactly as inv_rho/m_fluxes already do in the same
function. Diff checked with git apply --check (audit/notes/T5-scratch/014.diff):
--- a/src/projection/incflo_apply_cc_projection.cpp
+++ b/src/projection/incflo_apply_cc_projection.cpp
@@ -341,14 +341,14 @@ void incflo::ApplyCCProjection (Vector<MultiFab const*> density,
}
}
- Vector<MultiFab*> cc_phi(finest_level+1);
- Vector<MultiFab*> cc_gphi(finest_level+1);
+ Vector<MultiFab> cc_phi(finest_level+1);
+ Vector<MultiFab> cc_gphi(finest_level+1);
for (int lev = 0; lev <= finest_level; ++lev )
{
- cc_phi[lev] = new MultiFab(grids[lev], dmap[lev], 1, 1, MFInfo(), Factory(lev));
- cc_gphi[lev] = new MultiFab(grids[lev], dmap[lev], AMREX_SPACEDIM, 0, MFInfo(), Factory(lev));
- cc_phi[lev]->setVal(0.);
- cc_gphi[lev]->setVal(0.);
+ cc_phi[lev].define(grids[lev], dmap[lev], 1, 1, MFInfo(), Factory(lev));
+ cc_gphi[lev].define(grids[lev], dmap[lev], AMREX_SPACEDIM, 0, MFInfo(), Factory(lev));
+ cc_phi[lev].setVal(0.);
+ cc_gphi[lev].setVal(0.);
}
Vector<Array<MultiFab*,AMREX_SPACEDIM> > mac_vec(finest_level+1);
@@ -382,7 +382,7 @@ void incflo::ApplyCCProjection (Vector<MultiFab const*> density,
//
// Perform MAC projection: - del dot (dt/rho) grad phi = div(U)
//
- macproj->project(cc_phi,m_mac_mg_rtol,m_mac_mg_atol);
+ macproj->project(GetVecOfPtrs(cc_phi),m_mac_mg_rtol,m_mac_mg_atol);
//
// After the projection we grab the dt/rho (grad phi) used in the projection
@@ -403,9 +403,9 @@ void incflo::ApplyCCProjection (Vector<MultiFab const*> density,
// Note that "fluxes" comes back as MINUS (dt/rho) Gphi
//
#ifdef AMREX_USE_EB
- macproj->getFluxes(amrex::GetVecOfArrOfPtrs(m_fluxes), cc_phi, MLMG::Location::FaceCentroid);
+ macproj->getFluxes(amrex::GetVecOfArrOfPtrs(m_fluxes), GetVecOfPtrs(cc_phi), MLMG::Location::FaceCentroid);
#else
- macproj->getFluxes(amrex::GetVecOfArrOfPtrs(m_fluxes), cc_phi, MLMG::Location::FaceCenter);
+ macproj->getFluxes(amrex::GetVecOfArrOfPtrs(m_fluxes), GetVecOfPtrs(cc_phi), MLMG::Location::FaceCenter);
#endif
for (int lev=0; lev <= finest_level; ++lev)
@@ -413,7 +413,7 @@ void incflo::ApplyCCProjection (Vector<MultiFab const*> density,
#ifdef AMREX_USE_EB
amrex::Abort("Haven't written mac_to_ccvel for EB");
#else
- average_mac_to_ccvel(GetArrOfPtrs(m_fluxes[lev]),*cc_gphi[lev]);
+ average_mac_to_ccvel(GetArrOfPtrs(m_fluxes[lev]),cc_gphi[lev]);
#endif
}
@@ -427,8 +427,8 @@ void incflo::ApplyCCProjection (Vector<MultiFab const*> density,
Box const& tbx = mfi.tilebox();
Array4<Real> const& gp_cc = ld.gp.array(mfi);
Array4<Real> const& p_cc = ld.p_cc.array(mfi);
- Array4<Real const> const& gphi = cc_gphi[lev]->const_array(mfi);
- Array4<Real const> const& phi = cc_phi[lev]->const_array(mfi);
+ Array4<Real const> const& gphi = cc_gphi[lev].const_array(mfi);
+ Array4<Real const> const& phi = cc_phi[lev].const_array(mfi);
Array4<Real> const& u = ld.velocity.array(mfi);
Array4<Real const> const& rho = density[lev]->const_array(mfi);
Severity: medium · Category: memory-ub
Locations:
src/projection/incflo_apply_cc_projection.cpp:344-352,src/projection/incflo_apply_cc_projection.cpp:385,src/projection/incflo_apply_cc_projection.cpp:406-408,src/projection/incflo_apply_cc_projection.cpp:416,src/projection/incflo_apply_cc_projection.cpp:430-431Based on commit
46de3367(line numbers refer to that tree).The defect
(lines 344-349). There is no
deleteanywhere in the function (or the file:grep -n delete src/projection/*.cppis empty), so two full-domainMultiFabs (1 + AMREX_SPACEDIM components) leak on every call.
Prior-round issue #191 reported exactly this. PR #213 (
25a2d140, "fix issues 171 176 181 188 189 191",closes #191) touched thisfile, but its only hunk is the relocation of the U*-U^n subtraction for #181;
git diff 7307d872..HEAD -- src/projection/incflo_apply_cc_projection.cpp | grep -n "new MultiFab\|delete"returns nothing. The issue was closedwithout the fix landing.
Why it matters
ApplyCCProjectionruns once per predictor (and once per corrector with MOL) plus the initial projection and each initial iteration. Withincflo.use_cc_proj = 1the resident memory grows by (1+AMREX_SPACEDIM) cell-centred fields per projection call; on GPU this is devicememory, so a long run eventually fails with an out-of-memory error in the Arena. The nodal path uses projector-owned storage and has no such leak.
How to reach it
Any input with
incflo.use_cc_proj = 1(amr.max_level = 0is enforced for it). No shipped test sets it. DIM 2 or 3, non-EB (EB aborts at line 414 beforethe leak matters). CPU or GPU; on GPU the leak is device memory.
Suggested fix
Hold the temporaries in
Vector<MultiFab>and hand pointers to Hydro withGetVecOfPtrs, exactly asinv_rho/m_fluxesalready do in the samefunction. Diff checked with
git apply --check(audit/notes/T5-scratch/014.diff):