Fix various memory handling issues.

Delete things that were allocated, be clear about ownership.
This commit is contained in:
Ryan Curtin
2020-03-22 15:42:25 -04:00
parent b4e37680b2
commit 31c29505df
7 changed files with 66 additions and 30 deletions
@@ -80,6 +80,9 @@ template<typename TMetricType, typename ElemType>
HollowBallBound<TMetricType, ElemType>& HollowBallBound<TMetricType, ElemType>::
operator=(const HollowBallBound& other)
{
if (ownsMetric)
delete metric;
radii = other.radii;
center = other.center;
hollowCenter = other.hollowCenter;
+4 -2
View File
@@ -73,10 +73,12 @@ class BRNN
BRNN(const size_t rho,
const bool single = false,
OutputLayerType outputLayer = OutputLayerType(),
MergeLayerType mergeLayer = MergeLayerType(),
MergeOutputType mergeOutput = MergeOutputType(),
MergeLayerType* mergeLayer = new MergeLayerType(),
MergeOutputType* mergeOutput = new MergeOutputType(),
InitializationRuleType initializeRule = InitializationRuleType());
~BRNN();
/**
* Check if the optimizer has MaxIterations() parameter, if it does
* then check if it's value is less than the number of datapoints
+28 -8
View File
@@ -39,13 +39,13 @@ BRNN<OutputLayerType, MergeLayerType, MergeOutputType,
const size_t rho,
const bool single,
OutputLayerType outputLayer,
MergeLayerType mergeLayer,
MergeOutputType mergeOutput,
MergeLayerType* mergeLayer,
MergeOutputType* mergeOutput,
InitializationRuleType initializeRule) :
rho(rho),
outputLayer(std::move(outputLayer)),
mergeLayer(new MergeLayerType(mergeLayer)),
mergeOutput(new MergeOutputType(mergeOutput)),
mergeLayer(mergeLayer),
mergeOutput(mergeOutput),
initializeRule(std::move(initializeRule)),
inputSize(0),
outputSize(0),
@@ -60,6 +60,22 @@ BRNN<OutputLayerType, MergeLayerType, MergeOutputType,
/* Nothing to do here. */
}
template<typename OutputLayerType, typename MergeLayerType,
typename MergeOutputType, typename InitializationRuleType,
typename... CustomLayers>
BRNN<OutputLayerType, MergeLayerType, MergeOutputType,
InitializationRuleType, CustomLayers...>::~BRNN()
{
// Remove mergeLayer from the forward and backward RNNs so it doesn't get
// deleted. This assumes that mergeLayer is the last layer!
forwardRNN.network.pop_back();
backwardRNN.network.pop_back();
// Clean up layers that we allocated.
boost::apply_visitor(DeleteVisitor(), mergeLayer);
boost::apply_visitor(DeleteVisitor(), mergeOutput);
}
template<typename OutputLayerType, typename MergeLayerType,
typename MergeOutputType, typename InitializationRuleType,
typename... CustomLayers>
@@ -230,7 +246,7 @@ void BRNN<OutputLayerType, MergeLayerType, MergeOutputType,
boost::apply_visitor(LoadOutputParameterVisitor(results2),
backwardRNN.network.back());
boost::apply_visitor(ForwardVisitor(std::move(input),
boost::apply_visitor(ForwardVisitor(input,
boost::apply_visitor(outputParameterVisitor, mergeLayer)),
mergeLayer);
boost::apply_visitor(ForwardVisitor(
@@ -318,15 +334,15 @@ double BRNN<OutputLayerType, MergeLayerType, MergeOutputType,
boost::apply_visitor(LoadOutputParameterVisitor(results2),
backwardRNN.network.back());
boost::apply_visitor(ForwardVisitor(std::move(input),
boost::apply_visitor(ForwardVisitor(input,
boost::apply_visitor(outputParameterVisitor, mergeLayer)),
mergeLayer);
boost::apply_visitor(ForwardVisitor(
boost::apply_visitor(outputParameterVisitor, mergeLayer),
boost::apply_visitor(outputParameterVisitor, mergeOutput)),
mergeOutput);
performance += outputLayer.Forward(std::move(
boost::apply_visitor(outputParameterVisitor, mergeOutput)),
performance += outputLayer.Forward(
boost::apply_visitor(outputParameterVisitor, mergeOutput),
arma::mat(responses.slice(responseSeq).colptr(begin),
responses.n_rows, batchSize, false, true));
}
@@ -634,6 +650,8 @@ void BRNN<OutputLayerType, MergeLayerType, MergeOutputType,
{
if (!reset)
{
// TODO: what if we call ResetParameters() multiple times? Do we have to
// remove any existing mergeLayer?
boost::apply_visitor(AddVisitor<CustomLayers...>(
forwardRNN.network.back()), mergeLayer);
boost::apply_visitor(AddVisitor<CustomLayers...>(
@@ -707,6 +725,8 @@ void BRNN<OutputLayerType, MergeLayerType, MergeOutputType,
ar & BOOST_SERIALIZATION_NVP(parameter);
ar & BOOST_SERIALIZATION_NVP(backwardRNN);
ar & BOOST_SERIALIZATION_NVP(forwardRNN);
// TODO: are there more parameters to be serialized?
}
} // namespace ann
@@ -121,6 +121,8 @@ class HoeffdingTree
* @param dimensionMappings Mappings from dimension indices to positions in
* numeric and categorical split vectors. If left NULL, a new one will
* be created.
* @param copyDatasetInfo If true, then a copy of the datasetInfo will be
* made.
*/
HoeffdingTree(const data::DatasetInfo& datasetInfo,
const size_t numClasses,
@@ -133,7 +135,8 @@ class HoeffdingTree
const NumericSplitType<FitnessFunction>& numericSplitIn =
NumericSplitType<FitnessFunction>(0),
std::unordered_map<size_t, std::pair<size_t, size_t>>*
dimensionMappings = NULL);
dimensionMappings = NULL,
const bool copyDatasetInfo = true);
/**
* Construct a Hoeffding tree with no data and no information. Be sure to
@@ -96,7 +96,8 @@ HoeffdingTree<
categoricalSplitIn,
const NumericSplitType<FitnessFunction>& numericSplitIn,
std::unordered_map<size_t, std::pair<size_t, size_t>>*
dimensionMappingsIn) :
dimensionMappingsIn,
const bool copyDatasetInfo) :
dimensionMappings((dimensionMappingsIn != NULL) ? dimensionMappingsIn :
new std::unordered_map<size_t, std::pair<size_t, size_t>>()),
ownsMappings(dimensionMappingsIn == NULL),
@@ -105,8 +106,9 @@ HoeffdingTree<
maxSamples((maxSamples == 0) ? size_t(-1) : maxSamples),
checkInterval(checkInterval),
minSamples(minSamples),
datasetInfo(new data::DatasetInfo(datasetInfo)),
ownsInfo(true),
datasetInfo(copyDatasetInfo ? new data::DatasetInfo(datasetInfo) :
&datasetInfo),
ownsInfo(copyDatasetInfo),
successProbability(successProbability),
splitDimension(size_t(-1)),
majorityClass(0),
@@ -208,7 +210,18 @@ HoeffdingTree<FitnessFunction, NumericSplitType, CategoricalSplitType>::
{
// Copy each of the children.
for (size_t i = 0; i < other.children.size(); ++i)
{
children.push_back(new HoeffdingTree(*other.children[i]));
// Delete copied datasetInfo and dimension mappings.
delete children[i]->datasetInfo;
children[i]->datasetInfo = this->datasetInfo;
children[i]->ownsInfo = false;
delete children[i]->dimensionMappings;
children[i]->dimensionMappings = this->dimensionMappings;
children[i]->ownsMappings = false;
}
}
template<typename FitnessFunction,
@@ -744,7 +757,7 @@ void HoeffdingTree<
children.push_back(new HoeffdingTree(*datasetInfo, numClasses,
successProbability, maxSamples, checkInterval, minSamples,
CategoricalSplitType<FitnessFunction>(0, numClasses),
numericSplits[0], dimensionMappings));
numericSplits[0], dimensionMappings, false));
}
else if (numericSplits.size() == 0)
{
@@ -752,14 +765,14 @@ void HoeffdingTree<
children.push_back(new HoeffdingTree(*datasetInfo, numClasses,
successProbability, maxSamples, checkInterval, minSamples,
categoricalSplits[0], NumericSplitType<FitnessFunction>(numClasses),
dimensionMappings));
dimensionMappings, false));
}
else
{
// Pass both splits that we already have.
children.push_back(new HoeffdingTree(*datasetInfo, numClasses,
successProbability, maxSamples, checkInterval, minSamples,
categoricalSplits[0], numericSplits[0], dimensionMappings));
categoricalSplits[0], numericSplits[0], dimensionMappings, false));
}
children[i]->MajorityClass() = childMajorities[i];
@@ -874,7 +887,8 @@ void HoeffdingTree<
{
// The child doesn't actually own its own DatasetInfo. We do. The same
// applies for the dimension mappings.
children[i]->ownsInfo = false;
if (children[i]->datasetInfo == datasetInfo)
children[i]->ownsInfo = false;
children[i]->ownsMappings = false;
}
@@ -135,6 +135,12 @@ void HoeffdingTreeModel::BuildModel(
const size_t bins,
const size_t observationsBeforeBinning)
{
// Clean memory, if needed.
delete giniHoeffdingTree;
delete giniBinaryTree;
delete infoHoeffdingTree;
delete infoBinaryTree;
// Depending on the type, create the tree.
switch (type)
{
@@ -188,30 +188,18 @@ class HoeffdingTreeModel
data::DatasetInfo info;
if (type == GINI_HOEFFDING)
{
// Create fake tree to load into if needed.
if (Archive::is_loading::value)
giniHoeffdingTree = new GiniHoeffdingTreeType(info, 1, 1);
ar & BOOST_SERIALIZATION_NVP(giniHoeffdingTree);
}
else if (type == GINI_BINARY)
{
// Create fake tree to load into if needed.
if (Archive::is_loading::value)
giniBinaryTree = new GiniBinaryTreeType(info, 1, 1);
ar & BOOST_SERIALIZATION_NVP(giniBinaryTree);
}
else if (type == INFO_HOEFFDING)
{
// Create fake tree to load into if needed.
if (Archive::is_loading::value)
infoHoeffdingTree = new InfoHoeffdingTreeType(info, 1, 1);
ar & BOOST_SERIALIZATION_NVP(infoHoeffdingTree);
}
else if (type == INFO_BINARY)
{
// Create fake tree to load into if needed.
if (Archive::is_loading::value)
infoBinaryTree = new InfoBinaryTreeType(info, 1, 1);
ar & BOOST_SERIALIZATION_NVP(infoBinaryTree);
}
}