Skip to content

Commit

Permalink
Bugfix: ARKODE Stepper SetDefaults memory leak fix (#560)
Browse files Browse the repository at this point in the history 
Updated stepper `SetDefaults` routines to deallocate pre-existing
structures before nullifying, and moved initial creation of the
`SUNAdaptController` into stepper `SetDefaults` routines instead of
`arkCreate`. This allows non-adaptive steppers to leave the
SUNAdaptController object unallocated.

---------

Co-authored-by: Steven Roberts <[email protected]>
Co-authored-by: David Gardner <[email protected]>
  • Loading branch information
@drreynolds @Steven-Roberts @gardner48
3 people authored Sep 11, 2024
1 parent 9f0f337 commit 8d3eff4
Show file tree
Hide file tree
Showing 8 changed files with 198 additions and 91 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ Fixed the loading of ARKStep's default first order explicit method.
Fixed a CMake bug regarding usage of missing "print_warning" macro
that was only triggered when the deprecated `CUDA_ARCH` option was used.

Fixed a memory leak that could occur if ``ARKodeSetDefaults`` is called
repeatedly.

Fixed compilation errors when building the Trilinos Teptra NVector with CUDA
support.

Expand Down
3 changes: 3 additions & 0 deletions doc/shared/RecentChanges.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ Fixed the loading of ARKStep's default first order explicit method.
Fixed a CMake bug regarding usage of missing "print_warning" macro
that was only triggered when the deprecated ``CUDA_ARCH`` option was used.

Fixed a memory leak that could occur if :c:func:`ARKodeSetDefaults` is called
repeatedly.

Fixed compilation errors when building the Trilinos Teptra NVector with CUDA
support.

Expand Down
46 changes: 18 additions & 28 deletions src/arkode/arkode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1364,7 +1364,6 @@ void ARKodePrintMem(void* arkode_mem, FILE* outfile)
ARKodeMem arkCreate(SUNContext sunctx)
{
int iret;
long int lenrw, leniw;
ARKodeMem ark_mem;

if (!sunctx)
Expand Down Expand Up @@ -1485,21 +1484,6 @@ ARKodeMem arkCreate(SUNContext sunctx)
ark_mem->lrw += ARK_ADAPT_LRW;
ark_mem->liw += ARK_ADAPT_LIW;

/* Allocate default step controller (PID) and note storage */
ark_mem->hadapt_mem->hcontroller = SUNAdaptController_PID(sunctx);
if (ark_mem->hadapt_mem->hcontroller == NULL)
{
arkProcessError(NULL, ARK_MEM_FAIL, __LINE__, __func__, __FILE__,
"Allocation of step controller object failed");
ARKodeFree((void**)&ark_mem);
return (NULL);
}
ark_mem->hadapt_mem->owncontroller = SUNTRUE;
(void)SUNAdaptController_Space(ark_mem->hadapt_mem->hcontroller, &lenrw,
&leniw);
ark_mem->lrw += lenrw;
ark_mem->liw += leniw;

/* Initialize the interpolation structure to NULL */
ark_mem->interp = NULL;
ark_mem->interp_type = ARK_INTERP_HERMITE;
Expand All @@ -1523,7 +1507,7 @@ ARKodeMem arkCreate(SUNContext sunctx)
ark_mem->h = ZERO;
ark_mem->h0u = ZERO;

/* Set default values for integrator optional inputs */
/* Set default values for integrator and stepper optional inputs */
iret = ARKodeSetDefaults(ark_mem);
if (iret != ARK_SUCCESS)
{
Expand Down Expand Up @@ -1706,12 +1690,15 @@ int arkInit(ARKodeMem ark_mem, sunrealtype t0, N_Vector y0, int init_type)
ark_mem->tolsf = ONE;

/* Reset error controller object */
retval = SUNAdaptController_Reset(ark_mem->hadapt_mem->hcontroller);
if (retval != SUN_SUCCESS)
if (ark_mem->hadapt_mem->hcontroller)
{
arkProcessError(ark_mem, ARK_CONTROLLER_ERR, __LINE__, __func__, __FILE__,
"Unable to reset error controller object");
return (ARK_CONTROLLER_ERR);
retval = SUNAdaptController_Reset(ark_mem->hadapt_mem->hcontroller);
if (retval != SUN_SUCCESS)
{
arkProcessError(ark_mem, ARK_CONTROLLER_ERR, __LINE__, __func__,
__FILE__, "Unable to reset error controller object");
return (ARK_CONTROLLER_ERR);
}
}

/* Adaptivity counters */
Expand Down Expand Up @@ -2543,13 +2530,16 @@ int arkCompleteStep(ARKodeMem ark_mem, sunrealtype dsm)
ark_mem->fn_is_current = SUNFALSE;

/* Notify time step controller object of successful step */
retval = SUNAdaptController_UpdateH(ark_mem->hadapt_mem->hcontroller,
ark_mem->h, dsm);
if (retval != SUN_SUCCESS)
if (ark_mem->hadapt_mem->hcontroller)
{
arkProcessError(ark_mem, ARK_CONTROLLER_ERR, __LINE__, __func__, __FILE__,
"Failure updating controller object");
return (ARK_CONTROLLER_ERR);
retval = SUNAdaptController_UpdateH(ark_mem->hadapt_mem->hcontroller,
ark_mem->h, dsm);
if (retval != SUN_SUCCESS)
{
arkProcessError(ark_mem, ARK_CONTROLLER_ERR, __LINE__, __func__, __FILE__,
"Failure updating controller object");
return (ARK_CONTROLLER_ERR);
}
}

/* update scalar quantities */
Expand Down
12 changes: 11 additions & 1 deletion src/arkode/arkode_adapt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,10 @@ void arkPrintAdaptMem(ARKodeHAdaptMem hadapt_mem, FILE* outfile)
fprintf(outfile, " ark_hadapt: stability function data pointer = %p\n",
hadapt_mem->estab_data);
}
(void)SUNAdaptController_Write(hadapt_mem->hcontroller, outfile);
if (hadapt_mem->hcontroller != NULL)
{
(void)SUNAdaptController_Write(hadapt_mem->hcontroller, outfile);
}
}
}

Expand All @@ -98,6 +101,13 @@ int arkAdapt(ARKodeMem ark_mem, ARKodeHAdaptMem hadapt_mem, N_Vector ycur,
sunrealtype h_acc, h_cfl, int_dir;
int controller_order;

/* Return with no stepsize adjustment if the controller is NULL */
if (hadapt_mem->hcontroller == NULL)
{
ark_mem->eta = ONE;
return (ARK_SUCCESS);
}

/* Request error-based step size from adaptivity controller */
if (hadapt_mem->pq == 0)
{
Expand Down
62 changes: 59 additions & 3 deletions src/arkode/arkode_arkstep_io.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,8 @@ int arkStep_SetUserData(ARKodeMem ark_mem, void* user_data)
int arkStep_SetDefaults(ARKodeMem ark_mem)
{
ARKodeARKStepMem step_mem;
sunindextype Blrw, Bliw;
long int lenrw, leniw;
int retval;

/* access ARKodeARKStepMem structure */
Expand All @@ -677,12 +679,66 @@ int arkStep_SetDefaults(ARKodeMem ark_mem)
step_mem->msbp = MSBP; /* max steps between updates to J or P */
step_mem->stages = 0; /* no stages */
step_mem->istage = 0; /* current stage */
step_mem->Be = NULL; /* no Butcher tables */
step_mem->Bi = NULL;
step_mem->NLS = NULL; /* no nonlinear solver object */
step_mem->jcur = SUNFALSE;
step_mem->convfail = ARK_NO_FAILURES;
step_mem->stage_predict = NULL; /* no user-supplied stage predictor */

/* Remove pre-existing Butcher tables */
if (step_mem->Be)
{
ARKodeButcherTable_Space(step_mem->Be, &Bliw, &Blrw);
ark_mem->liw -= Bliw;
ark_mem->lrw -= Blrw;
ARKodeButcherTable_Free(step_mem->Be);
}
step_mem->Be = NULL;
if (step_mem->Bi)
{
ARKodeButcherTable_Space(step_mem->Bi, &Bliw, &Blrw);
ark_mem->liw -= Bliw;
ark_mem->lrw -= Blrw;
ARKodeButcherTable_Free(step_mem->Bi);
}
step_mem->Bi = NULL;

/* Remove pre-existing nonlinear solver object */
if (step_mem->NLS && step_mem->ownNLS) { SUNNonlinSolFree(step_mem->NLS); }
step_mem->NLS = NULL;

/* Remove pre-existing SUNAdaptController object, and replace with "PID" */
if (ark_mem->hadapt_mem->owncontroller)
{
retval = SUNAdaptController_Space(ark_mem->hadapt_mem->hcontroller, &lenrw,
&leniw);
if (retval == SUN_SUCCESS)
{
ark_mem->liw -= leniw;
ark_mem->lrw -= lenrw;
}
retval = SUNAdaptController_Destroy(ark_mem->hadapt_mem->hcontroller);
ark_mem->hadapt_mem->owncontroller = SUNFALSE;
if (retval != SUN_SUCCESS)
{
arkProcessError(ark_mem, ARK_MEM_FAIL, __LINE__, __func__, __FILE__,
"SUNAdaptController_Destroy failure");
return (ARK_MEM_FAIL);
}
}
ark_mem->hadapt_mem->hcontroller = SUNAdaptController_PID(ark_mem->sunctx);
if (ark_mem->hadapt_mem->hcontroller == NULL)
{
arkProcessError(ark_mem, ARK_MEM_FAIL, __LINE__, __func__, __FILE__,
"SUNAdaptControllerPID allocation failure");
return (ARK_MEM_FAIL);
}
ark_mem->hadapt_mem->owncontroller = SUNTRUE;
retval = SUNAdaptController_Space(ark_mem->hadapt_mem->hcontroller, &lenrw,
&leniw);
if (retval == SUN_SUCCESS)
{
ark_mem->liw += leniw;
ark_mem->lrw += lenrw;
}
return (ARK_SUCCESS);
}

Expand Down
43 changes: 27 additions & 16 deletions src/arkode/arkode_erkstep_io.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,23 +259,42 @@ int erkStep_SetRelaxFn(ARKodeMem ark_mem, ARKRelaxFn rfn, ARKRelaxJacFn rjac)
int erkStep_SetDefaults(ARKodeMem ark_mem)
{
ARKodeERKStepMem step_mem;
int retval;
sunindextype Blrw, Bliw;
long int lenrw, leniw;
int retval;

/* access ARKodeERKStepMem structure */
retval = erkStep_AccessStepMem(ark_mem, __func__, &step_mem);
if (retval != ARK_SUCCESS) { return (retval); }

/* Remove current SUNAdaptController object, and replace with "PI" */
retval = SUNAdaptController_Space(ark_mem->hadapt_mem->hcontroller, &lenrw,
&leniw);
if (retval == SUN_SUCCESS)
/* Set default values for integrator optional inputs */
step_mem->q = Q_DEFAULT; /* method order */
step_mem->p = 0; /* embedding order */
step_mem->stages = 0; /* no stages */
ark_mem->hadapt_mem->etamxf = SUN_RCONST(0.3); /* max change on error-failed step */
ark_mem->hadapt_mem->safety = SUN_RCONST(0.99); /* step adaptivity safety factor */
ark_mem->hadapt_mem->growth = SUN_RCONST(25.0); /* step adaptivity growth factor */

/* Remove pre-existing Butcher table */
if (step_mem->B)
{
ark_mem->liw -= leniw;
ark_mem->lrw -= lenrw;
ARKodeButcherTable_Space(step_mem->B, &Bliw, &Blrw);
ark_mem->liw -= Bliw;
ark_mem->lrw -= Blrw;
ARKodeButcherTable_Free(step_mem->B);
}
step_mem->B = NULL;

/* Remove current SUNAdaptController object, and replace with "PI" */
if (ark_mem->hadapt_mem->owncontroller)
{
retval = SUNAdaptController_Space(ark_mem->hadapt_mem->hcontroller, &lenrw,
&leniw);
if (retval == SUN_SUCCESS)
{
ark_mem->liw -= leniw;
ark_mem->lrw -= lenrw;
}
retval = SUNAdaptController_Destroy(ark_mem->hadapt_mem->hcontroller);
ark_mem->hadapt_mem->owncontroller = SUNFALSE;
if (retval != SUN_SUCCESS)
Expand All @@ -302,15 +321,7 @@ int erkStep_SetDefaults(ARKodeMem ark_mem)
ark_mem->lrw += lenrw;
}

/* Set default values for integrator optional inputs
(overwrite some adaptivity params for ERKStep use) */
step_mem->q = Q_DEFAULT; /* method order */
step_mem->p = 0; /* embedding order */
step_mem->stages = 0; /* no stages */
step_mem->B = NULL; /* no Butcher table */
ark_mem->hadapt_mem->etamxf = SUN_RCONST(0.3); /* max change on error-failed step */
ark_mem->hadapt_mem->safety = SUN_RCONST(0.99); /* step adaptivity safety factor */
ark_mem->hadapt_mem->growth = SUN_RCONST(25.0); /* step adaptivity growth factor */
/* Update some controller parameters */
(void)SUNAdaptController_SetErrorBias(ark_mem->hadapt_mem->hcontroller,
SUN_RCONST(1.2));
(void)SUNAdaptController_SetParams_PI(ark_mem->hadapt_mem->hcontroller,
Expand Down
Loading

0 comments on commit 8d3eff4

Please sign in to comment.