Skip to content

Commit

Permalink
Issue716 memory leaks in dynamics (#722)
Browse files Browse the repository at this point in the history
# memory leaks in dynamics

Fixes #716 

# Change Description

With #674 integrated, all dynamic buffers held by modules should get
freed before the program terminates. This update replaces raw pointers
with `std::unique_ptr` for heap buffers that are used internally by the
dynamics.

# Test Description
A manual test was done by running the dynamics benchmark for 1 time step
with valgrind:
```
==74249== LEAK SUMMARY:
==74249==    definitely lost: 0 bytes in 0 blocks
==74249==    indirectly lost: 0 bytes in 0 blocks
==74249==      possibly lost: 7,600 bytes in 19 blocks
==74249==    still reachable: 110,406 bytes in 717 blocks
==74249==         suppressed: 0 bytes in 0 blocks

```

The setup that was used is not suited for an automated test as it is
takes too long (init + 1 step on 32x32 grid takes upwards of 30mins).
  • Loading branch information
Thanduriel authored Nov 12, 2024
2 parents 3288d6c + 0a0a719 commit d9a5be0
Show file tree
Hide file tree
Showing 6 changed files with 13 additions and 14 deletions.
2 changes: 1 addition & 1 deletion dynamics/src/CGDynamicsKernel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ void CGDynamicsKernel<DGadvection>::initialise(
DynamicsKernel<DGadvection, DGstressComp>::initialise(coords, isSpherical, mask);

//! Initialize the parametric momentum map
pmap = new ParametricMomentumMap<CGdegree>(*smesh);
pmap = std::make_unique<ParametricMomentumMap<CGdegree>>(*smesh);
pmap->InitializeLumpedCGMassMatrix();
pmap->InitializeDivSMatrices();

Expand Down
2 changes: 1 addition & 1 deletion dynamics/src/include/BBMDynamicsKernel.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ template <int DGadvection> class BBMDynamicsKernel : public BrittleCGDynamicsKer
void initialise(const ModelArray& coords, bool isSpherical, const ModelArray& mask) override
{
BrittleCGDynamicsKernel<DGadvection>::initialise(coords, isSpherical, mask);
bbmStressStep.setPMap(pmap);
bbmStressStep.setPMap(pmap.get());
bbmStressStep.setDamage(damage);
}

Expand Down
4 changes: 2 additions & 2 deletions dynamics/src/include/BrittleCGDynamicsKernel.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ template <int DGadvection> class BrittleCGDynamicsKernel : public CGDynamicsKern
CGDynamicsKernel<DGadvection>::initialise(coords, isSpherical, mask);

//! Initialize stress transport
stresstransport = new Nextsim::DGTransport<DGstressComp>(*smesh);
stresstransport = std::make_unique<Nextsim::DGTransport<DGstressComp>>(*smesh);
stresstransport->settimesteppingscheme("rk2");

damage.resize_by_mesh(*smesh);
Expand Down Expand Up @@ -169,7 +169,7 @@ template <int DGadvection> class BrittleCGDynamicsKernel : public CGDynamicsKern
StressUpdateStep<DGadvection, DGstressComp>& stressStep;
const MEBParameters& params;

Nextsim::DGTransport<DGstressComp>* stresstransport;
std::unique_ptr<DGTransport<DGstressComp>> stresstransport;

DGVector<DGadvection> damage;

Expand Down
7 changes: 2 additions & 5 deletions dynamics/src/include/CGDynamicsKernel.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,7 @@ class CGDynamicsKernel : public DynamicsKernel<DGadvection, DGstressComp> {
using typename DynamicsKernel<DGadvection, DGstressComp>::DataMap;

public:
CGDynamicsKernel()
: pmap(nullptr)
{
}
CGDynamicsKernel() { }
virtual ~CGDynamicsKernel() = default;
void initialise(const ModelArray& coords, bool isSpherical, const ModelArray& mask) override;
void setData(const std::string& name, const ModelArray& data) override;
Expand Down Expand Up @@ -73,7 +70,7 @@ class CGDynamicsKernel : public DynamicsKernel<DGadvection, DGstressComp> {
CGVector<CGdegree> uAtmos;
CGVector<CGdegree> vAtmos;

ParametricMomentumMap<CGdegree>* pmap;
std::unique_ptr<ParametricMomentumMap<CGdegree>> pmap;
};

} /* namespace Nextsim */
Expand Down
10 changes: 6 additions & 4 deletions dynamics/src/include/DynamicsKernel.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "include/gridNames.hpp"

#include <map>
#include <memory>
#include <set>
#include <string>
#include <unordered_map>
Expand All @@ -44,7 +45,8 @@ template <int DGadvection, int DGstress> class DynamicsKernel {
virtual void initialise(const ModelArray& coords, bool isSpherical, const ModelArray& mask)
{
//! Define the spatial mesh
smesh = new ParametricMesh((isSpherical) ? Nextsim::SPHERICAL : Nextsim::CARTESIAN);
smesh = std::make_unique<ParametricMesh>(
(isSpherical) ? Nextsim::SPHERICAL : Nextsim::CARTESIAN);

smesh->coordinatesFromModelArray(coords);
if (isSpherical)
Expand All @@ -57,7 +59,7 @@ template <int DGadvection, int DGstress> class DynamicsKernel {
}

//! Initialize transport
dgtransport = new Nextsim::DGTransport<DGadvection>(*smesh);
dgtransport = std::make_unique<Nextsim::DGTransport<DGadvection>>(*smesh);
dgtransport->settimesteppingscheme("rk2");

// resize DG vectors
Expand Down Expand Up @@ -167,7 +169,7 @@ template <int DGadvection, int DGstress> class DynamicsKernel {
}

protected:
Nextsim::DGTransport<DGadvection>* dgtransport;
std::unique_ptr<Nextsim::DGTransport<DGadvection>> dgtransport;

DGVector<DGadvection> hice;
DGVector<DGadvection> cice;
Expand All @@ -182,7 +184,7 @@ template <int DGadvection, int DGstress> class DynamicsKernel {

double deltaT;

Nextsim::ParametricMesh* smesh;
std::unique_ptr<Nextsim::ParametricMesh> smesh;

virtual void updateMomentum(const TimestepTime& tst) = 0;

Expand Down
2 changes: 1 addition & 1 deletion dynamics/src/include/MEVPDynamicsKernel.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ template <int DGadvection> class MEVPDynamicsKernel : public VPCGDynamicsKernel<
void initialise(const ModelArray& coords, bool isSpherical, const ModelArray& mask) override
{
CGDynamicsKernel<DGadvection>::initialise(coords, isSpherical, mask);
MEVPStressStep.setPMap(pmap);
MEVPStressStep.setPMap(pmap.get());
}

private:
Expand Down

0 comments on commit d9a5be0

Please sign in to comment.