Handling Inter Class Memory Deallocation in C++

Question

I am implementing Artificial Neural Network in C++ for my Machine Learning course. Here's my code for "Neuron" -

struct Neuron
{
    float *inputs;          // pointer to inputs array
    int inputCount;         // number of input
    float *weights;
    float sum;              // linear weight sum of inputs
    float output = 0.0;     // output from activation function

    float bias = 1.0;

    float delta;            // error gradient


    Neuron(){}

    Neuron(int inputCount)
    {
        this->inputCount = inputCount;
        this->weights = new float [inputCount];

        InitializeWeights();
    }


    float ActivationFunction(float x)
    {
        return 1.0 / (1.0 + exp(-x));
    }

    void Calculate(float *inputs)
    {
        this->inputs = inputs;

        float temp = 0.0;

        for(int i=0; i<inputCount; i++)
        {
            temp += weights[i] * inputs[i];
        }

        temp += bias;
        sum = temp;
        output = ActivationFunction(sum);
    }

    void InitializeWeights()
    {
        for(int i=0; i<inputCount; i++)
        {
            weights[i] = rand() % 101 / 100;
        }
    }

    ~Neuron()
    {
        delete[] weights;
    }

};

I also have another struct called "Layer" which represents a layer. The neurons are initialized there as -

for(int i=0; i<neuronCount; i++)
{
    neurons[i] = Neuron(inputCount);
}

where "neuronCount" is the number of neurons in the layer. The problem is that the weight array in the neurons is immediately deallocated because of destructor calling. What should I do to prevent this? More importantly, is there any better way to design my program?

Answer 1

There are several things wrong with your code.

The first is that you failed to initialize your input and weights pointers in the default constructor. Thus the following simple program would cause an issue:

int main()
{
  Neuron n;
}

Since the destructor for n would be attempting to call delete [] on an uninitialized pointer weights.

The other problem is that your Neuron class is not safely copyable or assignable. The following program will also show problems with memory leaks and double deletion errors:

int main()
{
    Neuron n(10);
    Neuron n2 = n;  // copy constructor
    Neuron n3(2);
    n = n3;         // assignment
}   // memory leaks, double deletion issues once main() exits.

The third issue is that you're passing a pointer to float in your Calculate function, and you have no idea if the pointer is valid inside of that function, or even if it is valid, how many items the inputs pointer is representing. If the number is less than the number of weights, your code has a memory overrun attempting to access inputs[i] once i goes out-of-bounds of the input.

To fix these issues, the easiest thing to do is to use std::vector:

#include <vector>
#include <algorithm>
#include <cmath>
#include <cstdlib>

struct Neuron
{
    std::vector<float> inputs;          // pointer to inputs array
    std::vector<float> weights;
    float sum = 0;              // linear weight sum of inputs
    float output = 0.0;     // output from activation function
    float bias = 1.0;
    float delta = 0;            // error gradient

    Neuron() {}
    Neuron(int inputCount) : weights(inputCount) 
    {
        InitializeWeights();
    }

    float ActivationFunction(float x)
    {
        return 1.0 / (1.0 + exp(-x));
    }

    void Calculate(const std::vector<float>& inputs_)
    {
        inputs = inputs_;

        // make sure vectors are the same size.  If inputs is smaller,
        // the new elements added will be 0
        inputs.resize(weights.size());

        // use inner_product to get the sum of the products.
        sum = std::inner_product(std::begin(weights), 
                                 std::end(weights), 
                                 std::begin(inputs), 0.0f) + bias;

        output = ActivationFunction(sum);
    }

    void InitializeWeights()
    {
        std::generate(std::begin(weights), std::end(weights), rand() % 101 / 100);
    }
};

Note we no longer use pointers, as the std::vector takes care of the memory management. We also don't need to have member variables that keep track of the count (such as inputCount), since a vector knows its own size by using vector::size().

In addition, the std::inner_product is used to generate the sum in the Calculate function (after resizing the input vector to the same size as weights). The inputs_ is passed to Calculate as a std::vector, thus you can call it using a brace initialized list:

someInstance.Calculate({9.8, 5.6, 4.5}); // calls Calculate with a vector consisting of 3 items.

Also, the InitializeWeights function calls the std::generate algorithm function to set the weights vector.

---

If you were not to use std::vector, then your class would require a copy constructor and assignment operator, following the Rule of 3. I won't go over how you would fix your class without using vector, as the answer would get more involved.

---

The last item is that usage of rand() in C++ should be replaced with usage of the C++11 random number facilities.