GAutoFilter memory leak

[skn123]

Mike, I am trying to use the latest code in the repo and I observe this bug. I am unable to run the GAutoFilter snippet that you have provided here http://uaf46365.ddns.uark.edu/waffles/docs/supervised.html

It says that there is some corruption in the stack. Then I reverted back to a slightly older version and the code ran without any problems. I think changes in GLearner.cpp may have caused some problems here. Essentially, there is some memory leak that is happening in the newer code. Can you please check this?

[mikegashler]

Thanks, that led me to a memory leak, which I fixed. However, I did not see any stack corruption, and the issue I found was not caused by any recent changes, so I am not very confident that I found the same issue. If it helps, here is the code I used to find the memory leak, and now works as expected:

#include <exception>
#include <iostream>
#include <GClasses/GApp.h>
#include <GClasses/GError.h>
#include <GClasses/GNaiveBayes.h>

using namespace GClasses;
using std::cout;

int main(int argc, char *argv[])
{
    GApp::enableFloatingPointExceptions();

    GMatrix data;
    data.loadArff("iris.arff");
    GDataColSplitter splitter(data, 1);
    GMatrix& features = splitter.features();
    GMatrix& labels = splitter.labels();

    GNaiveBayes model;
    GAutoFilter af(&model, false);
    af.train(features, labels);
    GVec testIn(features.cols());
    testIn.fill(1.2345);
    GVec testOut(labels.cols());
    af.predict(testIn, testOut);
    testOut.print(cout);

    return 0;
}

[skn123]

I modified your code and it did not work. Here is the screenshot and the code

#include <exception>
#include <iostream>
#include <GApp.h>
#include <GError.h>
#include <GNaiveBayes.h>
#include <GHolders.h>
#include <GLearner.h>

using namespace GClasses;
using std::cout;

GClasses::Holder<GClasses::GTransducer> gSupLearner;

void InstantiateModel()
{
  GClasses::Holder<GClasses::GNaiveBayes> pMNBC;
  pMNBC.reset(new GClasses::GNaiveBayes());
  if (pMNBC.get()->canGeneralize())
    gSupLearner.reset(new GClasses::GAutoFilter(static_cast<GClasses::GSupervisedLearner*>(pMNBC.get())));
  else
    gSupLearner.reset(pMNBC.get());
}

int main(int argc, char *argv[])
{
    GApp::enableFloatingPointExceptions();
    InstantiateModel();
    GClasses::GSupervisedLearner* gModel = static_cast<GClasses::GSupervisedLearner*>(gSupLearner.get());

    GMatrix data;
    data.loadArff("iris.arff");
    GDataColSplitter splitter(data, 1);
    GMatrix& features = splitter.features();
    GMatrix& labels = splitter.labels();

    gModel->train(features, labels);
    return 0;
}

Notice, I am trying to create the objects as smart pointers (GHolder), though I would prefer them to be std::shared_ptr<> objects. I think the problem is with the destructor of GAutofilter

[mikegashler]

The GHolder class is not a smart pointer. It does not do ref-counting. It is more like std::unique_ptr. It is a bug to have multiple instances reference the same object. (I should probably rip this class out, since it is would be better to just use std::unique_ptr.)

The smart_ptr class in GHolder.h is a smart-pointer, like std::shared_ptr. It does ref-counting, and it is okay to have multiple instances reference the same object. (Again, however, it would be better to just use std::shared_ptr.)

If you search and replace "pMNBC.get()" in your code with "pMNBC.release()", I think that fixes the problem.

[skn123]

The modification you suggested partially works. It works if I place it in a main file (as above). However, if I define GClasses::HolderGClasses::GTransducer gSupLearner in a header file, within a class it again crashes. If I disable the AutoFilter part it works fine. Can you share an example that actually depicts the use of this function in a class structure and not from a main file? I am kind of stuck right now and I think I will revert back to an older version of the code before these changes to GAutoFilter were made.

I reverted back to this version: 3e11f88cec192ed88172cff48f8f05229b031ae0 and it is working in my environment without any crashes for GAutoFilter. Maybe you may have to do some regression tests beyond this version when you made changes to GAutoFilter

[mikegashler]

If you can provide code that works with older Waffles, but does not work now, I would be interested in investigating the cause. The following demo seems to work for me. Does it meet your needs?

// -------- LearnerWrapper.h
#ifndef LEARNERWRAPPER_H
#define LEARNERWRAPPER_H

#include <GClasses/GHolders.h>
#include <GClasses/GLearner.h>

class LearnerWrapper
{
protected:
    GClasses::Holder<GClasses::GSupervisedLearner> m_hSupLearner;

public:
    LearnerWrapper();
    ~LearnerWrapper();
    GClasses::GSupervisedLearner& get();
    void InstantiateModel();
};

#endif

// -------- LearnerWrapper.cpp
#include "LearnerWrapper.h"
#include <GClasses/GNaiveBayes.h>

using namespace GClasses;

LearnerWrapper::LearnerWrapper()
{
}

LearnerWrapper::~LearnerWrapper()
{
}

GClasses::GSupervisedLearner& LearnerWrapper::get()
{
    return *m_hSupLearner.get();
}

void LearnerWrapper::InstantiateModel()
{
    GSupervisedLearner* pNB = new GNaiveBayes();
    GAutoFilter* pAF = new GAutoFilter(pNB, true);
    m_hSupLearner.reset(pAF);
}

// -------- main.cpp
#include <exception>
#include <iostream>
#include "LearnerWrapper.h"
#include <GClasses/GApp.h>
#include <GClasses/GError.h>
#include <GClasses/GLearner.h>

using namespace GClasses;

int main(int argc, char *argv[])
{
    GApp::enableFloatingPointExceptions();

    LearnerWrapper lw;
    lw.InstantiateModel();
    GSupervisedLearner& model = lw.get();

    GMatrix data;
    data.loadArff("iris.arff");
    GDataColSplitter splitter(data, 1);
    GMatrix& features = splitter.features();
    GMatrix& labels = splitter.labels();

    model.train(features, labels);
    return 0;
}

[skn123]

Here is the code that I am using. I would like your comments on a.) Am I actually doing something insanely wrong? b.) If not, then is there any other way I can improve it?

You can use this as a template. The code is crashing if I instantiate the pipeline and I exit the program. I have backtracked it to this part of the code (if I am instantiating only NBC);

There are some other data structures with a "waffles_" tag. You can ignore them as they are mostly used to set parameters.

 gSupLearner.reset(new GClasses::GAutoFilter(static_cast<GClasses::GNaiveBayes*>(pMNBC.release())));

The difference that I noticed was that you have added something in the destructor of GAutoFilter which was not there in an older version of the code.

.cpp file

/*! \brief EnableBoosting
 *  This function checks if Boosting can be enabled
 */
template <class T>
void WafflesInterface::EnableBoosting(T* pSupLearner, std::string* tRes, std::string* base, void* mB)
{
  typedef GClasses::GSupervisedLearner  GSupervisedLearner;
  typedef GClasses::GResamplingAdaBoost GAdaBoost;
  waffles_Boost* mBoost = static_cast<waffles_Boost*>(mB);

  // Check if algorithm can generalize; Only then enable Boosting
  if (pSupLearner->canGeneralize())
  {
    GSupervisedLearner* pLearner = static_cast<GSupervisedLearner*>(pSupLearner);
    GClasses::Holder<GClasses::GTransducer> pAlg(new GClasses::GAutoFilter(pLearner));
    pMAdaBoost.reset(new GAdaBoost((GSupervisedLearner*)pAlg.get(), true, new GClasses::GLearnerLoader()));
    pMAdaBoost.get()->setTrainSize(mBoost->m_TrainSizeRatio);
    pMAdaBoost.get()->setSize(mBoost->m_EnsembleSize);
  }
  else
  {
    // Boosting cannot be done in this algorithm; Safely omit this from the
    // pipeline. Indicate this for future use
    *tRes = (*base);
  }
}//end of function

/*! \brief BuildSingleClassifierPipeline
 *  This function builds up a single classifier pipeline
 */
template <class T>
bool WafflesInterface::BuildSingleClassifierPipeline( T* mSingle, std::string* tRes)
{
  bool classifierFound = false;
  std::string base = "";
  typedef GClasses::GResamplingAdaBoost GAdaBoost;

  if ((*tRes)=="NBC" || (*tRes) == "bNBC")// Naive Bayes Pipeline
  {
    classifierFound = true;
    GClasses::Holder<GClasses::GNaiveBayes> pMNBC(new GClasses::GNaiveBayes());

    if (mSingle->m_NaiveBayes.aboost.m_Enable==1)
    {
      base = "NBC";
      EnableBoosting<GClasses::GNaiveBayes>(pMNBC.release(), tRes, &base, &mSingle->m_NaiveBayes.aboost);
      if (pMAdaBoost.get()->canGeneralize())
        gSupLearner.reset(new GClasses::GAutoFilter(static_cast<GClasses::GSupervisedLearner*>(pMAdaBoost.release())));
      else
        classifierFound = false;
    }
    else
    {
      if (pMNBC.get()->canGeneralize())
        gSupLearner.reset(new GClasses::GAutoFilter(static_cast<GClasses::GNaiveBayes*>(pMNBC.release())));
      else
        classifierFound = false;
    }
  }
  else
  if ((*tRes)=="KNN" || (*tRes) == "bKNN")// KNN Pipeline
  {
    classifierFound = true;
    GClasses::Holder<GClasses::GKNN> pMKNN(new GClasses::GKNN());
    pMKNN.get()->setNeighborCount(mSingle->m_KNN.m_Neighbors);

    if (mSingle->m_KNN.aboost.m_Enable==1)
    {
      base = "KNN";
      EnableBoosting<GClasses::GKNN>(pMKNN.release(), tRes, &base, &mSingle->m_KNN.aboost);
      if (pMAdaBoost.get()->canGeneralize())
        gSupLearner.reset(new GClasses::GAutoFilter(static_cast<GClasses::GSupervisedLearner*>(pMAdaBoost.release())));
      else
        classifierFound = false;
    }
    else
    {
      if (pMKNN.get()->canGeneralize())
        gSupLearner.reset(new GClasses::GAutoFilter(static_cast<GClasses::GSupervisedLearner*>(pMKNN.release())));
      else
        classifierFound = false;
    }
  }
  else
  if ((*tRes)=="GP" || (*tRes) == "bGP")// Gaussian Process pipeline
  {
    classifierFound = true;
    GClasses::Holder<GClasses::GGaussianProcess> pMGP(new GClasses::GGaussianProcess());

    if (mSingle->m_GaussianProcess.aboost.m_Enable==1)
    {
      base = "GP";
      EnableBoosting<GClasses::GGaussianProcess>(pMGP.release(), tRes, &base, &mSingle->m_GaussianProcess.aboost);
      if (pMAdaBoost.get()->canGeneralize())
        gSupLearner.reset(new GClasses::GAutoFilter(static_cast<GClasses::GSupervisedLearner*>(pMAdaBoost.release())));
      else
        classifierFound = false;
    }
    else
    {
      if (pMGP.get()->canGeneralize())
        gSupLearner.reset(new GClasses::GAutoFilter(static_cast<GClasses::GSupervisedLearner*>(pMGP.release())));
      else
        classifierFound = false;
    }
  }
  else
  if ((*tRes)=="RF" || (*tRes) == "bRF")// Random forest pipeline
  {
    classifierFound = true;
    GClasses::Holder<GClasses::GRandomForest> pMRF(new GClasses::GRandomForest(mSingle->m_RandomForest.m_Trees, mSingle->m_RandomForest.m_Samples));

    if (mSingle->m_RandomForest.aboost.m_Enable==1)
    {
      base = "RF";
      EnableBoosting<GClasses::GRandomForest>(pMRF.release(), tRes, &base, &mSingle->m_RandomForest.aboost);
      if (pMAdaBoost.get()->canGeneralize())
        gSupLearner.reset(new GClasses::GAutoFilter(static_cast<GClasses::GSupervisedLearner*>(pMAdaBoost.release())));
      else
        classifierFound = false;
    }
    else
    {
      if (pMRF.get()->canGeneralize())
        gSupLearner.reset(new GClasses::GAutoFilter(static_cast<GClasses::GSupervisedLearner*>(pMRF.release())));
      else
        classifierFound = false;
    }
  }
  else
  // Neural network pipeline
  if ((*tRes)=="NNet" || (*tRes) == "bNNet")
  {
    classifierFound = true;
    GClasses::Holder<GClasses::GNeuralNet> pMNNet(new GClasses::GNeuralNet());
    pMNNet.get()->addLayer(new GClasses::GLayerClassic(0, mSingle->m_NeuralNetwork.m_AddLayer));
    pMNNet.get()->setLearningRate(mSingle->m_NeuralNetwork.m_LearningRate);
    pMNNet.get()->setMomentum(mSingle->m_NeuralNetwork.m_Momentum);
    pMNNet.get()->setWindowSize(mSingle->m_NeuralNetwork.m_WindowEpochs);
    pMNNet.get()->setImprovementThresh(mSingle->m_NeuralNetwork.m_MinWindowImprovement);
    pMNNet.get()->setValidationPortion(mSingle->m_NeuralNetwork.m_Holdout);
    pMNNet.get()->addLayer(new GClasses::GLayerClassic(0,0));

    if (mSingle->m_NeuralNetwork.aboost.m_Enable==1)
    {
      base = "NNet";
      EnableBoosting<GClasses::GNeuralNet>(pMNNet.release(), tRes, &base, &mSingle->m_NeuralNetwork.aboost);
      if (pMAdaBoost.get()->canGeneralize())
        gSupLearner.reset(new GClasses::GAutoFilter(static_cast<GClasses::GSupervisedLearner*>(pMAdaBoost.release())));
      else
        classifierFound = false;
    }
    else
    {
      if (pMNNet.get()->canGeneralize())
        gSupLearner.reset(new GClasses::GAutoFilter(static_cast<GClasses::GSupervisedLearner*>(pMNNet.release())));
      else
        classifierFound = false;
    }
  }
  else
  // Decision tree pipeline
  if ((*tRes)=="DTree" || (*tRes) == "bDTree")
  {
    classifierFound = true;
    GClasses::Holder<GClasses::GDecisionTree> pMDTree(new GClasses::GDecisionTree());
    if (mSingle->m_DecisionTree.m_Binary == 1) pMDTree.get()->useBinaryDivisions();
    pMDTree.get()->useRandomDivisions(mSingle->m_DecisionTree.m_Random);
    pMDTree.get()->setLeafThresh(mSingle->m_DecisionTree.m_LeafThresh);
    pMDTree.get()->setMaxLevels(mSingle->m_DecisionTree.m_MaxLevels);

    if (mSingle->m_DecisionTree.aboost.m_Enable==1)
    {
      base = "DTree";
      EnableBoosting<GClasses::GDecisionTree>(pMDTree.release(), tRes, &base, &mSingle->m_DecisionTree.aboost);
      if (pMAdaBoost.get()->canGeneralize())
        gSupLearner.reset(new GClasses::GAutoFilter(static_cast<GClasses::GSupervisedLearner*>(pMAdaBoost.release())));
      else
        classifierFound = false;
    }
    else
    {
      if (pMDTree.get()->canGeneralize())
        gSupLearner.reset(new GClasses::GAutoFilter(static_cast<GClasses::GSupervisedLearner*>(pMDTree.get())));
      else
        classifierFound = false;
    }
  }

  return classifierFound;
}//end of function

/*! \brief BuildEnsembleClassifierPipeline
 *  This function builds up an ensemble classifier pipeline
 */
template <class T1, class T2>
void WafflesInterface::BuildEnsembleClassifierPipeline( T1* pEnsemble, T2* mSingle, std::vector<std::string>* tRes, bool isBag)
{
  // Based on the classifiers chosen, we will build the pipeline
  for (auto iAlgo : (*tRes))
  {
    if ( (iAlgo != "waffles" ) && 
         (iAlgo != "Bag")      && 
         (iAlgo != "Bucket")   && 
         (iAlgo != "Single") )
    {
      bool classifierFound = BuildSingleClassifierPipeline<T2>( mSingle, &iAlgo );
      if (classifierFound)
      {
        pEnsemble->addLearner(new GClasses::GAutoFilter(static_cast<GClasses::GSupervisedLearner*>(gSupLearner.release())));
      }
      else
      {
        // delete the tag for future references
        iAlgo = "";
      }
    }
  }// finished adding weak classifiers to the ensemble

  // reset the value in gSupLearner
  gSupLearner.reset(pEnsemble);
}//end of function

/*! \brief InstantiatePipeline
 *  This function builds up the pipeline that will be used to generate the
 *  training model.
 */
std::string WafflesInterface::InstantiatePipeline( void* itX)
{
  typedef GClasses::GBag GBag;
  typedef GClasses::GBucket GBucket;

  typedef std::map<int, ClassificationParams*>::iterator AlgoIterator;
  AlgoIterator* it = static_cast<AlgoIterator*>(itX);

  // We will build a pipeline depending on what algorithm is selected in the
  // Tag
  std::vector<std::string> tRes;
  boost::split(tRes, (*it)->second->m_Name, boost::is_any_of("_"));

  // Now, check the second value in the tag. Based on this, we will make a choice
  // of either buiding a single, or an ensemble classifier pipeline
  if (tRes[1] == "Single")
  {
    BuildSingleClassifierPipeline<waffles_Single>(&((*it)->second->m_wafflesParams.m_Single),
                                                  &tRes[2]);
  }
  else
  if (tRes[1] == "Bag")
  {
    GClasses::Holder<GBag> pEnsemble(new GBag());
    pEnsemble.get()->setWorkerThreads(4);
    BuildEnsembleClassifierPipeline<GBag, waffles_Bag>( pEnsemble.release(), &((*it)->second->m_wafflesParams.m_Bag),
                                                        &tRes, true);
    pEnsemble.release();
  }
  else
  if (tRes[1] == "Bucket")
  {
    GClasses::Holder<GBucket> pEnsemble(new GBucket());
    BuildEnsembleClassifierPipeline<GBucket, waffles_Bucket>( pEnsemble.release(), &((*it)->second->m_wafflesParams.m_Bucket),
                                                              &tRes, false);
    pEnsemble.release();
  }

  // we rebuild the Tag
  std::string f_Tag = tRes[0];
  for (auto iTag: tRes)
  {
    if ( (iTag != "waffles") && (iTag !=""))
    {
        f_Tag += "_"+iTag;
    }
  }
  tRes.clear();
  return f_Tag;
}//end of function

and here is the .h file

#ifndef WAFFLESINTERFACE_H_
#define WAFFLESINTERFACE_H_
#include <string>
#include <vector>
#include "Utilities/LinearAlgebra/MatrixTypes.h"
#include "Common/Callback.h"
//GClasses
#include "GClasses/GLearner.h"
#include "GClasses/GHolders.h"
#include "GClasses/GEnsemble.h"

/*! \brief class WafflesInterface
 *  This class is the main gateway for interfacing Waffles routines with the
 *  integrated pipeline.
 */
class WafflesInterface
{
  public:
    WafflesInterface() {}
    ~WafflesInterface(){}
#ifdef TRAIN
    /*! \brief Instantiate the pipeline and return a tag associated with the pipeline
    */
    std::string InstantiatePipeline(void* itX);
    /*! \brief Training: Implements the training phase of multi-class classification
     *  routine.
     */
    int Training( RMMatrix_Float* m_TrainingFeatures,
                  RMMatrix_Float* m_TrainingLabels,
                  bool* writeModel,
                  std::string& m_ModelFile, std::string* Tag,
                  void* itX);
#else
    /*! \brief Prediction: Implements the prediction phase of a multi-class
     *  classification routine.
     */
    void Prediction(void* mD, void* mM, void* mL);
    /*! \brief ReadModel */
    int  ReadModel(void* m_M, std::string& Tag, std::string& mModelFile);
#endif

    /*! \brief A callback */
    CallbackSignal status;
  private:
#ifdef TRAIN

    template <class T>
    bool BuildSingleClassifierPipeline(T* mSingle, std::string* tRes);
    template <class T1, class T2>
    void BuildEnsembleClassifierPipeline(T1* mEns, T2* mSingle, std::vector<std::string>* tRes, bool isbag);
    template <class T>
    void EnableBoosting(T* pSupLearner, std::string* tRes, std::string* base, void* mBoost);
#endif
  private:
#ifdef TRAIN
    /*! \brief Instances of All learners in Waffles*/
    GClasses::Holder<GClasses::GTransducer> gSupLearner;  
    GClasses::Holder<GClasses::GResamplingAdaBoost> pMAdaBoost;
#endif
};//end of class
#endif

[mikegashler]

I can't seem to get a repro on Linux. Perhaps it only occurs on Windows? Maybe we could find it by identifying the change that caused it. How far back do you have to go? The likely candidates seem to be: On 2015-10-16, I added a feature to GAutoFilter. Since you are not calling GAutoFilter::prefilterData, this change should have no effect. On 2015-10-10, I renamed some variables. This should have no effect. On 2015-10-08, I converted all of GClasses to using the GVec class. This change was too big to just examine, so it would help to know whether you have to go back to before or after it. On 2015-09-26, I added a feature to GAutoFilter, but I think you are not using it, so it should have no effect.

[skn123]

Mike, I am not sure if I can do the checks. However, I did post the commit # from which I pulled out the code. If you want, you can use this to check the changes you had made:

https://github.com/mikegashler/waffles/tree/3e11f88cec192ed88172cff48f8f05229b031ae0

[mikegashler]

Sorry for overlooking that. My bad. This definitely seems to point the finger at the change on 2015-10-16 (58d1eda0). However, I am having trouble understanding how that change could cause a problem. If you comment out these two lines in GClasses/GLearner.cpp,

GAutoFilter::~GAutoFilter()
{
    //for(size_t i = 0; i < m_prefilteredData.size(); i++)
    //  delete(m_prefilteredData[i]);
}

does that seem make the issue go away? (This is just a test, not a fix.)

[skn123]

Mike, I would be tempted to do that, but I am not sure if doing that would derail any other stuff (as in memory bloat). I can give it a shot, but for that I will have to create a new working branch :( However, I have tried to understand the rationale behind that destructor. If a class is encapsulated within GHolder (or std::shared_ptr), they should get automatically release when the variable goes out of scope. I think, the variable is somehow not going out of scope and an explicit destructor is being called!

[mikegashler]

You might try putting a breakpoint on the line

  delete(m_prefilteredData[i]);

I would expect that this line is never actually reached. If it is reached, the call-stack will tell us how the destructor is being called and what is going on.

[mikegashler]

If you want to make a branch, that is fine with me. If you're too busy, I totally understand. No pressure. My unit tests log performance, so we should notice if some change significantly bloats the memory footprint of the library. In other words, I don't think bloat is a concern, but we won't really know for sure until afterwards. I will probably do it eventually, if no one else does it first, but it might take a few years before I get to it.

On 12/07/2015 11:18 AM, skn123 wrote:

Mike, I would be tempted to do that, but I am not sure if doing that would derail any other stuff (as in memory bloat). I can give it a shot, but for that I will have to create a new working branch :(

— Reply to this email directly or view it on GitHub https://github.com/mikegashler/waffles/issues/5#issuecomment-162597646.

[skn123]

Fixed the bug..and it was from my end! Nothing to do with Waffles :)