From c137e70f72430d552dcf20037a7fa444329cc642 Mon Sep 17 00:00:00 2001
From: Giovanni Bussi <giovanni.bussi@gmail.com>
Date: Wed, 18 Apr 2018 16:21:07 +0200
Subject: [PATCH] vesselRegister().create() now returns a unique_ptr
Notice that this change required a bit of hacking.
setAveragingAction now takes a std::unique_ptr as an argument.
I think ownership is clearer now since it is explicitly declared.
---
src/adjmat/AdjacencyMatrixBase.cpp | 4 +++-
src/analysis/Average.cpp | 4 +++-
src/analysis/Histogram.cpp | 7 ++++---
src/gridtools/ActionWithGrid.cpp | 11 +++++++----
src/gridtools/ActionWithGrid.h | 6 ++++--
src/gridtools/ConvertToFES.cpp | 8 ++++----
src/gridtools/FindContourSurface.cpp | 4 ++--
src/gridtools/FindSphericalContour.cpp | 6 ++++--
src/gridtools/FourierTransform.cpp | 6 +++---
src/gridtools/InterpolateGrid.cpp | 7 ++++---
src/multicolvar/MultiColvarDensity.cpp | 8 +++++---
src/vesselbase/ActionWithAveraging.cpp | 5 +++--
src/vesselbase/ActionWithAveraging.h | 2 +-
src/vesselbase/ActionWithVessel.cpp | 15 ++++++---------
src/vesselbase/ActionWithVessel.h | 2 +-
src/vesselbase/VesselRegister.cpp | 5 ++---
src/vesselbase/VesselRegister.h | 7 ++++---
17 files changed, 60 insertions(+), 47 deletions(-)
diff --git a/src/adjmat/AdjacencyMatrixBase.cpp b/src/adjmat/AdjacencyMatrixBase.cpp
index 457b08791..cae178b0a 100644
--- a/src/adjmat/AdjacencyMatrixBase.cpp
+++ b/src/adjmat/AdjacencyMatrixBase.cpp
@@ -161,7 +161,9 @@ void AdjacencyMatrixBase::finishMatrixSetup( const bool& symmetric, const std::v
vesselbase::VesselOptions da("","",0,param,this);
Keywords keys; AdjacencyMatrixVessel::registerKeywords( keys );
- vesselbase::VesselOptions da2(da,keys); mat = new AdjacencyMatrixVessel(da2); addVessel( mat );
+ vesselbase::VesselOptions da2(da,keys);
+ std::unique_ptr<AdjacencyMatrixVessel> ves( new AdjacencyMatrixVessel(da2) );
+ addVessel( std::move( ves ) );
setupMultiColvarBase( all_atoms );
}
diff --git a/src/analysis/Average.cpp b/src/analysis/Average.cpp
index e2e61b2f9..7faa57102 100644
--- a/src/analysis/Average.cpp
+++ b/src/analysis/Average.cpp
@@ -127,7 +127,9 @@ Average::Average( const ActionOptions& ao ):
vesselbase::VesselOptions da("myaverage","",-1,instring,this);
Keywords keys; AverageVessel::registerKeywords( keys );
vesselbase::VesselOptions dar( da, keys );
- myaverage = new AverageVessel(dar); setAveragingAction( myaverage, false );
+ std::unique_ptr<AverageVessel> average( new AverageVessel(dar) );
+ myaverage = average.get();
+ setAveragingAction( std::move(average), false );
}
void Average::performOperations( const bool& from_update ) {
diff --git a/src/analysis/Histogram.cpp b/src/analysis/Histogram.cpp
index 705e7f606..0da49d077 100644
--- a/src/analysis/Histogram.cpp
+++ b/src/analysis/Histogram.cpp
@@ -312,7 +312,8 @@ Histogram::Histogram(const ActionOptions&ao):
}
}
// And create the grid
- createGrid( "histogram", vstring );
+ auto grid=createGrid( "histogram", vstring );
+ // notice that createGrid also sets mygrid=grid.get()
if( mygrid->getType()=="flat" ) {
if( mvectors ) error("computing histogram for three dimensional vectors but grid is not of fibonacci type - use CONCENTRATION");
std::vector<std::string> gmin( narg ), gmax( narg );
@@ -333,12 +334,12 @@ Histogram::Histogram(const ActionOptions&ao):
if( myvessels.size()>0 ) {
// Create a task list
for(unsigned i=0; i<myvessels[0]->getFullNumberOfTasks(); ++i) addTaskToList(i);
- setAveragingAction( mygrid, true );
+ setAveragingAction( std::move(grid), true );
} else {
// Create a task list
for(unsigned i=0; i<mygrid->getNumberOfPoints(); ++i) addTaskToList(i);
myhist->addOneKernelEachTimeOnly();
- setAveragingAction( mygrid, myhist->noDiscreteKernels() );
+ setAveragingAction( std::move(grid), myhist->noDiscreteKernels() );
}
checkRead();
}
diff --git a/src/gridtools/ActionWithGrid.cpp b/src/gridtools/ActionWithGrid.cpp
index 44d51dbcb..0b3011c51 100644
--- a/src/gridtools/ActionWithGrid.cpp
+++ b/src/gridtools/ActionWithGrid.cpp
@@ -41,7 +41,7 @@ ActionWithGrid::ActionWithGrid( const ActionOptions& ao):
{
}
-void ActionWithGrid::createGrid( const std::string& type, const std::string& inputstr ) {
+std::unique_ptr<GridVessel> ActionWithGrid::createGrid( const std::string& type, const std::string& inputstr ) {
// Start creating the input for the grid
std::string vstring = inputstr;
if( keywords.exists("KERNEL") ) {
@@ -57,15 +57,18 @@ void ActionWithGrid::createGrid( const std::string& type, const std::string& inp
vesselbase::VesselOptions da("mygrid","",-1,vstring,this);
Keywords keys; gridtools::AverageOnGrid::registerKeywords( keys );
vesselbase::VesselOptions dar( da, keys );
+ std::unique_ptr<GridVessel> grid;
if( type=="histogram" ) {
- mygrid = new HistogramOnGrid(dar);
+ grid.reset( new HistogramOnGrid(dar) );
} else if( type=="average" ) {
- mygrid = new AverageOnGrid(dar);
+ grid.reset( new AverageOnGrid(dar) );
} else if( type=="grid" ) {
- mygrid = new GridVessel(dar);
+ grid.reset( new GridVessel(dar) );
} else {
plumed_merror("no way to create grid of type " + type );
}
+ mygrid=grid.get();
+ return grid;
}
void ActionWithGrid::turnOnDerivatives() {
diff --git a/src/gridtools/ActionWithGrid.h b/src/gridtools/ActionWithGrid.h
index 86e676a59..cfa5da327 100644
--- a/src/gridtools/ActionWithGrid.h
+++ b/src/gridtools/ActionWithGrid.h
@@ -39,8 +39,10 @@ private:
protected:
/// The grid vessel
GridVessel* mygrid;
-/// Read in stuff that is specifically for the grid and create it
- void createGrid( const std::string& type, const std::string& inputstr );
+/// Read in stuff that is specifically for the grid and create it.
+/// Notice that this not only returns a unique_ptr but also set the protected
+/// member mygrid as an alias to that unique_ptr.
+ std::unique_ptr<GridVessel> createGrid( const std::string& type, const std::string& inputstr );
public:
static void registerKeywords( Keywords& keys );
explicit ActionWithGrid( const ActionOptions& );
diff --git a/src/gridtools/ConvertToFES.cpp b/src/gridtools/ConvertToFES.cpp
index 989e5e4f0..0eac068c0 100644
--- a/src/gridtools/ConvertToFES.cpp
+++ b/src/gridtools/ConvertToFES.cpp
@@ -92,11 +92,11 @@ ConvertToFES::ConvertToFES(const ActionOptions&ao):
plumed_assert( ingrid->getNumberOfComponents()==1 );
// Create a grid
- createGrid( "grid", "COMPONENTS=" + getLabel() + " " + ingrid->getInputString() );
- if( ingrid->noDerivatives() ) mygrid->setNoDerivatives();
+ auto grid=createGrid( "grid", "COMPONENTS=" + getLabel() + " " + ingrid->getInputString() );
+ if( ingrid->noDerivatives() ) grid->setNoDerivatives();
std::vector<double> fspacing;
- mygrid->setBounds( ingrid->getMin(), ingrid->getMax(), ingrid->getNbin(), fspacing);
- setAveragingAction( mygrid, true );
+ grid->setBounds( ingrid->getMin(), ingrid->getMax(), ingrid->getNbin(), fspacing);
+ setAveragingAction( std::move(grid), true );
simtemp=0.; parse("TEMP",simtemp);
if(simtemp>0) simtemp*=plumed.getAtoms().getKBoltzmann();
diff --git a/src/gridtools/FindContourSurface.cpp b/src/gridtools/FindContourSurface.cpp
index cce85179b..075c292c3 100644
--- a/src/gridtools/FindContourSurface.cpp
+++ b/src/gridtools/FindContourSurface.cpp
@@ -144,8 +144,8 @@ FindContourSurface::FindContourSurface(const ActionOptions&ao):
for(unsigned i=1; i<gdirs.size(); ++i) {
if( ingrid->isPeriodic(gdirs[i]) ) vstring+=",T"; else vstring+=",F";
}
- createGrid( "grid", vstring ); mygrid->setNoDerivatives();
- setAveragingAction( mygrid, true );
+ auto grid=createGrid( "grid", vstring ); grid->setNoDerivatives();
+ setAveragingAction( std::move(grid), true );
}
void FindContourSurface::clearAverage() {
diff --git a/src/gridtools/FindSphericalContour.cpp b/src/gridtools/FindSphericalContour.cpp
index 72e256b98..e57d2fce0 100644
--- a/src/gridtools/FindSphericalContour.cpp
+++ b/src/gridtools/FindSphericalContour.cpp
@@ -140,8 +140,10 @@ FindSphericalContour::FindSphericalContour(const ActionOptions&ao):
log.printf(" looking for contour in windows of length %f \n", (max-min)/nbins);
// Set this here so the same set of grid points are used on every turn
std::string vstring = "TYPE=fibonacci COMPONENTS=" + getLabel() + " COORDINATES=x,y,z PBC=F,F,F";
- createGrid( "grid", vstring ); mygrid->setNoDerivatives();
- setAveragingAction( mygrid, true ); mygrid->setupFibonacciGrid( npoints );
+ auto grid=createGrid( "grid", vstring ); grid->setNoDerivatives();
+ setAveragingAction( std::move(grid), true );
+ // use mygrid, since at this point grid has been moved
+ mygrid->setupFibonacciGrid( npoints );
checkRead();
// Create a task list
diff --git a/src/gridtools/FourierTransform.cpp b/src/gridtools/FourierTransform.cpp
index 05f4d9724..b3d151005 100644
--- a/src/gridtools/FourierTransform.cpp
+++ b/src/gridtools/FourierTransform.cpp
@@ -163,9 +163,9 @@ FourierTransform::FourierTransform(const ActionOptions&ao):
// Create a grid on which to store the fourier transform of the input grid
- createGrid( "grid", vstring );
- if( ingrid->noDerivatives() ) mygrid->setNoDerivatives();
- setAveragingAction( mygrid, false );
+ auto grid=createGrid( "grid", vstring );
+ if( ingrid->noDerivatives() ) grid->setNoDerivatives();
+ setAveragingAction( std::move(grid), false );
checkRead();
#endif
diff --git a/src/gridtools/InterpolateGrid.cpp b/src/gridtools/InterpolateGrid.cpp
index fc1d98c1d..70fcf48ed 100644
--- a/src/gridtools/InterpolateGrid.cpp
+++ b/src/gridtools/InterpolateGrid.cpp
@@ -76,7 +76,8 @@ InterpolateGrid::InterpolateGrid(const ActionOptions&ao):
plumed_assert( ingrid->getNumberOfComponents()==1 );
if( ingrid->noDerivatives() ) error("cannot interpolate a grid that does not have derivatives");
// Create the input from the old string
- createGrid( "grid", "COMPONENTS=" + getLabel() + " " + ingrid->getInputString() );
+ auto grid=createGrid( "grid", "COMPONENTS=" + getLabel() + " " + ingrid->getInputString() );
+ // notice that createGrid also sets mygrid=grid.get()
std::vector<unsigned> nbin; parseVector("GRID_BIN",nbin);
std::vector<double> gspacing; parseVector("GRID_SPACING",gspacing);
@@ -85,8 +86,8 @@ InterpolateGrid::InterpolateGrid(const ActionOptions&ao):
}
// Need this for creation of tasks
- mygrid->setBounds( ingrid->getMin(), ingrid->getMax(), nbin, gspacing );
- setAveragingAction( mygrid, true );
+ grid->setBounds( ingrid->getMin(), ingrid->getMax(), nbin, gspacing );
+ setAveragingAction( std::move(grid), true );
// Now create task list
for(unsigned i=0; i<mygrid->getNumberOfPoints(); ++i) addTaskToList(i);
diff --git a/src/multicolvar/MultiColvarDensity.cpp b/src/multicolvar/MultiColvarDensity.cpp
index b2941aa57..b44779881 100644
--- a/src/multicolvar/MultiColvarDensity.cpp
+++ b/src/multicolvar/MultiColvarDensity.cpp
@@ -228,10 +228,12 @@ MultiColvarDensity::MultiColvarDensity(const ActionOptions&ao):
// Create a task list
for(unsigned i=0; i<mycolv->getFullNumberOfTasks(); ++i) addTaskToList(i);
// And create the grid
- if( mycolv->isDensity() ) createGrid( "histogram", vstring );
- else createGrid( "average", vstring );
+ std::unique_ptr<gridtools::GridVessel> grid;
+ if( mycolv->isDensity() ) grid=createGrid( "histogram", vstring );
+ else grid=createGrid( "average", vstring );
+ mygrid=grid.get();
// And finish the grid setup
- setAveragingAction( mygrid, true );
+ setAveragingAction( std::move(grid), true );
// Enusre units for cube files are set correctly
if( !fractional ) {
diff --git a/src/vesselbase/ActionWithAveraging.cpp b/src/vesselbase/ActionWithAveraging.cpp
index 2ff6bd190..a0ddedad3 100644
--- a/src/vesselbase/ActionWithAveraging.cpp
+++ b/src/vesselbase/ActionWithAveraging.cpp
@@ -81,8 +81,9 @@ ActionWithAveraging::ActionWithAveraging( const ActionOptions& ao ):
}
}
-void ActionWithAveraging::setAveragingAction( AveragingVessel* av_vessel, const bool& usetasks ) {
- myaverage=av_vessel; addVessel( myaverage );
+void ActionWithAveraging::setAveragingAction( std::unique_ptr<AveragingVessel> av_vessel, const bool& usetasks ) {
+ myaverage=av_vessel.get();
+ addVessel( std::move(av_vessel) );
useRunAllTasks=usetasks; resizeFunctions();
}
diff --git a/src/vesselbase/ActionWithAveraging.h b/src/vesselbase/ActionWithAveraging.h
index 45ded46bc..7d482399c 100644
--- a/src/vesselbase/ActionWithAveraging.h
+++ b/src/vesselbase/ActionWithAveraging.h
@@ -65,7 +65,7 @@ protected:
/// The current weight and its logarithm
double lweight, cweight;
/// Set the averaging action
- void setAveragingAction( AveragingVessel* av_vessel, const bool& usetasks );
+ void setAveragingAction( std::unique_ptr<AveragingVessel> av_vessel, const bool& usetasks );
/// Check if we are using the normalization condition when calculating this quantity
bool noNormalization() const ;
public:
diff --git a/src/vesselbase/ActionWithVessel.cpp b/src/vesselbase/ActionWithVessel.cpp
index 3a8a66c1f..31d41772d 100644
--- a/src/vesselbase/ActionWithVessel.cpp
+++ b/src/vesselbase/ActionWithVessel.cpp
@@ -99,19 +99,16 @@ ActionWithVessel::~ActionWithVessel() {
void ActionWithVessel::addVessel( const std::string& name, const std::string& input, const int numlab ) {
VesselOptions da(name,"",numlab,input,this);
- Vessel* vv=vesselRegister().create(name,da);
- FunctionVessel* fv=dynamic_cast<FunctionVessel*>(vv);
+ std::unique_ptr<Vessel> vv=vesselRegister().create(name,da);
+ FunctionVessel* fv=dynamic_cast<FunctionVessel*>(vv.get());
if( fv ) {
std::string mylabel=Vessel::transformName( name );
plumed_massert( keywords.outputComponentExists(mylabel,false), "a description of the value calculated by vessel " + name + " has not been added to the manual");
}
- addVessel(vv);
+ addVessel(std::move(vv));
}
-void ActionWithVessel::addVessel( Vessel* vv ) {
-// logically, this function should accept a unique_ptr.
-// temporarily, we make here the conversion:
- std::unique_ptr<Vessel> vv_ptr(vv);
+void ActionWithVessel::addVessel( std::unique_ptr<Vessel> vv_ptr ) {
// In the original code, the dynamically casted pointer was deleted here.
// Now that vv_ptr is a unique_ptr, the object will be deleted automatically when
@@ -150,9 +147,9 @@ StoreDataVessel* ActionWithVessel::buildDataStashes( ActionWithVessel* actionTha
}
VesselOptions da("","",0,"",this);
- StoreDataVessel* mm=new StoreDataVessel(da);
+ std::unique_ptr<StoreDataVessel> mm( new StoreDataVessel(da) );
if( actionThatUses ) mm->addActionThatUses( actionThatUses );
- addVessel(mm);
+ addVessel(std::move(mm));
// Make sure resizing of vessels is done
resizeFunctions();
diff --git a/src/vesselbase/ActionWithVessel.h b/src/vesselbase/ActionWithVessel.h
index f6db97bbb..d4e454d22 100644
--- a/src/vesselbase/ActionWithVessel.h
+++ b/src/vesselbase/ActionWithVessel.h
@@ -107,7 +107,7 @@ protected:
std::vector<unsigned> taskFlags;
/// Add a vessel to the list of vessels
void addVessel( const std::string& name, const std::string& input, const int numlab=0 );
- void addVessel( Vessel* vv );
+ void addVessel( std::unique_ptr<Vessel> vv );
/// Add a bridging vessel to the list of vessels
BridgeVessel* addBridgingVessel( ActionWithVessel* tome );
/// Complete the setup of this object (this routine must be called after construction of ActionWithValue)
diff --git a/src/vesselbase/VesselRegister.cpp b/src/vesselbase/VesselRegister.cpp
index d8afaf2da..c9329847e 100644
--- a/src/vesselbase/VesselRegister.cpp
+++ b/src/vesselbase/VesselRegister.cpp
@@ -62,14 +62,13 @@ bool VesselRegister::check(std::string key) {
return false;
}
-Vessel* VesselRegister::create(std::string keyword, const VesselOptions&da) {
- Vessel* df;
+std::unique_ptr<Vessel> VesselRegister::create(std::string keyword, const VesselOptions&da) {
+ std::unique_ptr<Vessel> df;
if(check(keyword)) {
Keywords keys; mk[keyword](keys);
VesselOptions nda( da,keys );
df=m[keyword](nda);
}
- else df=NULL;
return df;
}
diff --git a/src/vesselbase/VesselRegister.h b/src/vesselbase/VesselRegister.h
index b589f7936..ccb14c97e 100644
--- a/src/vesselbase/VesselRegister.h
+++ b/src/vesselbase/VesselRegister.h
@@ -26,6 +26,7 @@
#include <cstring>
#include <vector>
#include <map>
+#include <memory>
#include "tools/Exception.h"
#include "tools/Keywords.h"
@@ -38,7 +39,7 @@ class VesselOptions;
class VesselRegister {
private:
/// Pointer to a function which, given the keyword for a distribution function, creates it
- typedef Vessel*(*creator_pointer)(const VesselOptions&);
+ typedef std::unique_ptr<Vessel>(*creator_pointer)(const VesselOptions&);
/// Pointer to the function that reserves the keyword for the distribution
typedef void(*keyword_pointer)(Keywords&);
/// The set of possible distribution functions we can work with
@@ -57,7 +58,7 @@ public:
/// Verify if a distribution keyword is present in the register
bool check(std::string keyname);
/// Create a distribution function of the specified type
- Vessel* create(std::string keyword, const VesselOptions&da);
+ std::unique_ptr<Vessel> create(std::string keyword, const VesselOptions&da);
/// Return the keywords
Keywords getKeywords();
};
@@ -66,7 +67,7 @@ VesselRegister& vesselRegister();
#define PLUMED_REGISTER_VESSEL(classname,keyword) \
static class classname##RegisterMe{ \
- static PLMD::vesselbase::Vessel * create(const PLMD::vesselbase::VesselOptions&da){return new classname(da);} \
+ static std::unique_ptr<PLMD::vesselbase::Vessel> create(const PLMD::vesselbase::VesselOptions&da){return std::unique_ptr<classname>( new classname(da) );} \
public: \
classname##RegisterMe(){PLMD::vesselbase::vesselRegister().add(keyword,create,classname::reserveKeyword,classname::registerKeywords);} \
~classname##RegisterMe(){PLMD::vesselbase::vesselRegister().remove(create);} \
--
GitLab