2
\$\begingroup\$

While reading an online book (Michael A. Nielsen, Neural Networks and Deep Learning, Determination Press, 2015) regarding neural networks, I decided I wanted to try and build a neural network which does not require a predefined network size, i.e. the layer depth and size are defined by input arguments.

My goal was to make the network object modular, such that different training principles can be appended afterwards. The main is then responsible for calling the modules such that it leads to training, testing or displaying of results.

I have tried programming with OOP concepts in mind. However, I am finding myself struggling with what should be handled by the NeuralNetwork object and is to be handled in Main. On stack overflow it is mentioned that an object should be responsible for all of its affairs, including the in/output. However, where do I draw the line? For instance, the network is responsible for storing and loading of results, yet it is not responsible for reading the parameter file stating the network size to load.

As a fairly inexperienced c++ programmer I welcome any and all insights to improve my skill.

The code is also in GitHub: https://github.com/vanderboog/mnist-neural-network

The manual can be found in the GitHub link.

Neural_Network.h

struct CLayer
{
    // Declaration of variables used in a each layer
    arma::dvec a;
    arma::dvec z;
    arma::dvec b;
    arma::dvec db;
    arma::dmat w;
    arma::dmat dw;
    arma::dvec kD;
};

class NeuralNetwork
{
    int numOfLayers_;
    int learnSetSize_;
    double learnRate_;
    double regularization_;
    double halfRegularization_;
    int learnReductionCycle_;
    double learnReductionFactor_;
    int iCountEpoch_;
    int digit_;
    std::string setSavePath_;

    //Smart pointers are used to ensure freeing of memory. The pointers are not always used and can therefore not be freed in a destructor
    std::unique_ptr<int[]> sizeLayer;
    std::unique_ptr<CLayer[]> pLayer;

public:
    arma::dvec cost;

    NeuralNetwork();

    void initializeLayers(int, int *, std::string);
    void setHyperParameters(double, double, double);
    void layerInfo();
    void training(const arma::dmat &, const arma::uvec &);
    arma::uvec yVectorGenerator(const arma::uword &);
    arma::dvec sigmoid(arma::dvec &);
    arma::dvec Dsigmoid(arma::dvec &);
    int computePerformance(const arma::dmat &, const arma::uvec &);
    int feedForward(const arma::dvec &);
    void setLearningReductionParameters(double, int);
    void reduceLearnRate(double);
    void storeResults();
    void loadResults(const std::string &, int, int *);
};

Neural_Network.cpp

#include <armadillo>
#include <iostream>
#include <memory>
#include <string>
#include "Neural_Network.h"

NeuralNetwork::NeuralNetwork() :
    learnSetSize_(100),
    learnReductionCycle_(1000),
    learnReductionFactor_(1),
    learnRate_(0.1),
    regularization_(0),
    halfRegularization_(regularization_ / 2),
    iCountEpoch_(0)
{}


void NeuralNetwork::initializeLayers(int numOfLayers, int *pLayerSize, std::string setSavePath)
{
    ///////////////////////////////////////////////////////
    /// Creates layers and sets component sizes.
    /// Layers are initialized ready for training
    //////////////////////////////////////////////////////
    setSavePath_ = setSavePath;
    numOfLayers_ = numOfLayers;
    sizeLayer = std::unique_ptr<int[]>(new int[numOfLayers_]);
    for (int iLayer = 0; iLayer < numOfLayers_; iLayer++)
    {
        sizeLayer[iLayer] = pLayerSize[iLayer];
    }

    /// Create the layers and initialize parameters;
    pLayer = std::unique_ptr<CLayer[]>(new CLayer[numOfLayers_]);
    pLayer[0].a.set_size(sizeLayer[0]); // Treat first layer different as it does not have b, w, nor kD
    for (int iLayer = 1; iLayer < numOfLayers_; iLayer++)
    {
        // Initialize: matrix and vector sizes
        pLayer[iLayer].a.set_size(sizeLayer[iLayer]);
        pLayer[iLayer].z.set_size(sizeLayer[iLayer]);
        pLayer[iLayer].b = arma::randn(sizeLayer[iLayer]);
        pLayer[iLayer].w.set_size(sizeLayer[iLayer], sizeLayer[iLayer - 1]);
        pLayer[iLayer].kD.set_size(sizeLayer[iLayer]);
        pLayer[iLayer].db = pLayer[iLayer].b;
        pLayer[iLayer].dw = pLayer[iLayer].w;

        /// Generate gaussian random generated values with standard deviation dependent on layer sizes.
        std::default_random_engine generator{static_cast<long unsigned int>(std::chrono::high_resolution_clock::now().time_since_epoch().count())}; // Use high precision time to determine random seed
        std::normal_distribution<double> distribution(0.0, sqrt((double)sizeLayer[iLayer - 1]));                                                    // Generate random values of with stdev based on incoming layer
        for (arma::uword iRow = 0; iRow < sizeLayer[iLayer]; iRow++)
        {
            for (arma::uword iCol = 0; iCol < sizeLayer[iLayer - 1]; iCol++)
            {
                pLayer[iLayer].w(iRow, iCol) = distribution(generator);
            }
        }
    }
}

void NeuralNetwork::setHyperParameters(double learnSetSize, double learnRate, double regularization)
{
    learnSetSize_ = learnSetSize;
    learnRate_ = learnRate;
    regularization_ = regularization;
    halfRegularization_ = regularization_ / 2;
    std::cout << "Hyper parameters settings:\n\t- Learning set size = " << learnSetSize_ << "\n\t- Learning parameter (learnRate_) = " << learnRate_ << "\n\t- Regularization_ parameter (lambda) = " << regularization_ << "\n";
}

void NeuralNetwork::layerInfo()
{
    /// Outputs layers information
    std::cout << "Number of layers: \t" << numOfLayers_ << "\n";
    // std::cout << "Number of neurons in layer 1: \t" << sizeLayer[0] << "\n";
    for (int iLayer = 0; iLayer < numOfLayers_; iLayer++)
    {
        std::cout << "Number of neurons in layer " << iLayer + 1 << ": \t" << sizeLayer[iLayer] << "\n";
    }

    for (int iLayer = 1; iLayer < numOfLayers_; iLayer++)
    {
        std::cout << "Weight matrix size (rows by cols) to layer " << iLayer + 1 << ": \t" << pLayer[iLayer].w.n_rows << " x " << pLayer[iLayer].w.n_cols << "\n";
    }
}

void NeuralNetwork::training(const arma::dmat &trainingSet, const arma::uvec &trainingLabels)
{
    ///////////////////////////////////////////////////////
    /// Training the neural network by feeding it one epoch
    ///////////////////////////////////////////////////////
    /// Initialize
    int numOfCol = trainingSet.n_cols;
    int numOfRow = trainingSet.n_rows;
    arma::uvec yVector(sizeLayer[numOfLayers_ - 1]);
    arma::uvec oneVector(sizeLayer[numOfLayers_ - 1], arma::fill::ones);
    arma::uvec sampleStack_i = arma::randperm(numOfCol);

    /// Reduce learnRate_ if -reduceLearnRate is used
    if(iCountEpoch_ % learnReductionCycle_ == 0 && iCountEpoch_ != 0)
    {
        reduceLearnRate(learnReductionFactor_);
    }

    int numOfCyclesPerEpoch = numOfCol / learnSetSize_; // Compute amount of cycles making up one epoch and only loop over complete cycles, omitting remaining samples
    /// Cycle through the epoch and apply learning after each cycle
    cost = arma::zeros(numOfCyclesPerEpoch);
    for (int iCycle = 0; iCycle < numOfCyclesPerEpoch; iCycle++)
    {
        int iSampleOffset = iCycle * learnSetSize_;

        /// Set dw and db to zero before each cycle
        for (int iLayer = 1; iLayer < numOfLayers_; iLayer++)
        {
            pLayer[iLayer].db.zeros(pLayer[iLayer].db.n_rows, pLayer[iLayer].db.n_cols);
            pLayer[iLayer].dw.zeros(pLayer[iLayer].dw.n_rows, pLayer[iLayer].dw.n_cols);
        }

        for (int iSample = 0; iSample < learnSetSize_; iSample++)
        {
            /// Load the image and create label vector (yVector)
            pLayer[0].a = trainingSet.col(sampleStack_i(iSample + iSampleOffset));
            yVector = yVectorGenerator(trainingLabels(sampleStack_i(iSample + iSampleOffset)));

            /// Feed forward
            digit_ = feedForward(pLayer[0].a);

            /// Compute cost (-= is used instead of -1*)
            cost[iCycle] -= as_scalar(trans(yVector) * log(pLayer[numOfLayers_ - 1].a) + trans(oneVector - yVector) * log(oneVector - pLayer[numOfLayers_ - 1].a));
            /// Add regularization_ term:
            if (regularization_ != 0)  // Skip overhead computation in case of 0
            {
                for (int iLayer = 1; iLayer < numOfLayers_; iLayer++)
                {
                    cost[iCycle] += halfRegularization_ * accu(pLayer[iLayer].w % pLayer[iLayer].w);  //Expensive
                }
            }

            /// Back propagation
            /// Compute error terms: dC/dz
            pLayer[numOfLayers_ - 1].kD = pLayer[numOfLayers_ - 1].a - yVector;
            for (int iLayer = numOfLayers_ - 2; iLayer > 0; iLayer--)
            {
                pLayer[iLayer].kD = pLayer[iLayer + 1].w.t() * pLayer[iLayer + 1].kD % Dsigmoid(pLayer[iLayer].z);
            }
            /// Compute gradient descent of w and b (seperate loop for clarity)
            for (int iLayer = 1; iLayer < numOfLayers_; iLayer++)
            {
                pLayer[iLayer].dw += arma::kron(pLayer[iLayer].kD, pLayer[iLayer - 1].a.t());
                pLayer[iLayer].db += pLayer[iLayer].kD;
            }
        }

        /// Apply gradient descent on w and b
        for (int iLayer = 1; iLayer < numOfLayers_; iLayer++)
        {
            pLayer[iLayer].w -= learnRate_ * (pLayer[iLayer].dw + regularization_ * pLayer[iLayer].w) / learnSetSize_; // with regularization_ term
            pLayer[iLayer].b -= learnRate_ * pLayer[iLayer].db / learnSetSize_;
        }

        cost = cost / learnSetSize_;
    }
    iCountEpoch_++;
}

arma::uvec NeuralNetwork::yVectorGenerator(const arma::uword &label)
{
    /// Generates a vector representation of the label: vector of zeros, with at the labelth index a 1
    arma::uvec y = arma::zeros<arma::uvec>(sizeLayer[numOfLayers_ - 1]);
    y(label) = 1;
    return y;
}

arma::dvec NeuralNetwork::sigmoid(arma::dvec &z)
{
    return 1 / (1 + exp(-z));
}

arma::dvec NeuralNetwork::Dsigmoid(arma::dvec &z)
{
    arma::dvec dS = sigmoid(z);
    return (dS % (1 - dS)); // %: Schur product, i.e. element-wise product
}

int NeuralNetwork::computePerformance(const arma::dmat &testSet, const arma::uvec &testLabels)
{
    ////////////////////////////////////////////
    /// Compute network performance based on the test set
    ////////////////////////////////////////////

    int iCountCorrect = 0;
    int sizeSet = testSet.n_cols;
    for (int iSample = 0; iSample < sizeSet; iSample++)
    {
        // Load testimage & apply feedforward. Count the correct answers
        if (feedForward(testSet.col(iSample)) == testLabels(iSample))
        {
            iCountCorrect++;
        }
    }
    std::cout << "Performance: " << iCountCorrect << " / " << sizeSet << "\n";
    return iCountCorrect;
}

int NeuralNetwork::feedForward(const arma::dvec &imVector)
{
    /// Apply feedforward to determine and return the network answer
    pLayer[0].a = imVector;
    for (int iLayer = 1; iLayer < numOfLayers_; iLayer++)
    {
        pLayer[iLayer].z = pLayer[iLayer].w * pLayer[iLayer - 1].a + pLayer[iLayer].b;
        pLayer[iLayer].a = sigmoid(pLayer[iLayer].z);
    }
    return pLayer[numOfLayers_ - 1].a.index_max();
}

void NeuralNetwork::setLearningReductionParameters(double learnReductionFactor, int learnReductionCycle)
{
    learnReductionFactor_ = learnReductionFactor;
    learnReductionCycle_ = learnReductionCycle;
    std::cout << "Learning rate reduction factor: " << learnReductionFactor_ << "\n";
    std::cout << "Learning rate reduction cycle: " << learnReductionCycle_ << "\n";
}

void NeuralNetwork::reduceLearnRate(double factor)
{
    learnRate_ = learnRate_ / factor;
    std::cout << "learnRate_ reduced to:\t" << learnRate_ << "\n";
}

void NeuralNetwork::storeResults()
{
    /// Store essential parameters of the network: weights and biases
    for (int iLayer = 1; iLayer < numOfLayers_; iLayer++)
    {
        pLayer[iLayer].w.save(setSavePath_ + "/w" + std::to_string(iLayer + 1));
        pLayer[iLayer].b.save(setSavePath_ + "/b" + std::to_string(iLayer + 1));
    }
}

void NeuralNetwork::loadResults(const std::string &setSavePath, int numOfLayers, int *layerSize)
{
    setSavePath_ = setSavePath;
    numOfLayers_ = numOfLayers;

    /// Load the actual stored data
    for (int iLayer = 1; iLayer < numOfLayers_; iLayer++)
    {
        std::cout << "Loading file: " << (setSavePath_ + "/w" + std::to_string(iLayer + 1)) << "\n";
        pLayer[iLayer].w.load(setSavePath_ + "/w" + std::to_string(iLayer + 1));
        std::cout << "Loading file: " << (setSavePath_ + "/b" + std::to_string(iLayer + 1)) << "\n";
        pLayer[iLayer].b.load(setSavePath_ + "/b" + std::to_string(iLayer + 1));
    }

    layerInfo();
}

Main.cpp

#include <armadillo>
#include <boost/filesystem.hpp>
#include <fstream>
#include <iomanip>
#include <iostream>
#include <memory>
#include <opencv2/highgui.hpp>
#include <random>
#include <string>
#include "Neural_Network.h"
#include "ReadMNIST.h"
#include "Visualization.h"


std::string setPathSave(std::string const setPath)
{
    /// Make sure Result_Network directory exists
    if (!boost::filesystem::exists(setPath))
    {
        boost::filesystem::create_directory(setPath);
    }

    /// Set save path to a unique path of 'Results_##', found by incrementing from 1 
    /// to 32. If the full range is used, the save path is set to 'Result_32'
    std::string setSavePath;
    for (int iFolder = 1; iFolder < 33; iFolder++)
    {
        setSavePath = setPath + "/Results_" + std::to_string(iFolder);
        if (!boost::filesystem::exists(setSavePath))
        {
            boost::filesystem::create_directory(setSavePath);
            break;
        }
    }

    std::cout << "Save path is set to: " << setSavePath << "\n";
    return setSavePath;
}

void showUsage()
{
    std::cout << std::left << std::setw(92) << "Options available in this program:" << std::endl;
    std::cout << std::setw(2) << "" << std::setw(18) << "-train" << std::setw(72) << "Train a new neural network. This mode requires the training set and " << std::endl;
    std::cout << std::setw(20) << "" << std::setw(72) << "labels. See training options below for more details." << std::endl;
    std::cout << std::setw(2) << "" << std::setw(18) << "-test" << std::setw(72) << "Test a trained network. This mode requires a trained network stored in " << std::endl;
    std::cout << std::setw(20) << "" << std::setw(72) << "Results_Network and the test set. After '-test' refer to the folder " << std::endl;
    std::cout << std::setw(20) << "" << std::setw(72) << "containing the results by the trailing number in the folder name, e.g." << std::endl;
    std::cout << std::setw(20) << "" << std::setw(72) << "'-test 1' to test the network in 'Network_Results/Results_1'. See test " << std::endl;
    std::cout << std::setw(20) << "" << std::setw(72) << "options below for more details.\n"
              << std::endl;

    std::cout << std::left << std::setw(92) << "Training options: " << std::endl;
    std::cout << std::setw(2)  << "" << std::setw(18) << "-layers" << std::setw(72) << "Set the total amount of layers and layer sizes used in the network," << std::endl;
    std::cout << std::setw(20) << "" << std::setw(72) << "including the input and output layer. After '-layers', the total number" << std::endl;
    std::cout << std::setw(20) << "" << std::setw(72) << "of layers is required. Thereafter, the layer size should be given in" << std::endl;
    std::cout << std::setw(20) << "" << std::setw(72) << "curly brackets, e.g. 'layers 3 {784,30,10}'." << std::endl;
    std::cout << std::setw(2)  << "" << std::setw(18) << "-param" << std::setw(72) << "Set learning hyperparameters. Parameters which are to be set are: batch" << std::endl;
    std::cout << std::setw(20) << "" << std::setw(72) << "size before learning step, learning rate, and the regularization" << std::endl;
    std::cout << std::setw(20) << "" << std::setw(72) << "parameter, respectively. In case no regularization is to be used, the" << std::endl;
    std::cout << std::setw(20) << "" << std::setw(72) << "parameter is to be set to zero, e.g, '-param {1000,0.1,0}'" << std::endl;
    std::cout << std::setw(2)  << "" << std::setw(18) << "-reduceLearning" << std::setw(72) << "Used to reduce the learning parameter by {factor x, per y epoch}," << std::endl;
    std::cout << std::setw(20) << "" << std::setw(72) << "e.g. -reduceLearning {2,20}.\n"
              << std::endl;

    std::cout << std::left << std::setw(92) << "Test options:" << std::endl;
    std::cout << std::setw(2)  << "" << std::setw(18) << "-display" << std::setw(72) << "Opens a window to visualize the test images in a random sequence." << std::endl;
    std::cout << std::setw(20) << "" << std::setw(72) << "Visualization can be stopped by pressing <q>." << std::endl;
}

int main(int argc, const char **argv)
{
    /// Test if sufficient arguments are given
    if (argc < 2)
    {
        std::cerr << "No arguments are given. Use --help to show options.\nTerminating program." << std::endl;
        return 1;
    }

    /// Initialize paths
    std::string const setPath = getCurrentDir(); // part of "readmnist.h"
    std::string const setPathTrainingImages = setPath + "/../Training_images/train-images.idx3-ubyte";
    std::string const setPathTrainingLabels = setPath + "/../Training_images/train-labels.idx1-ubyte";
    std::string const setPathTestImages = setPath + "/../Test_images/t10k-images.idx3-ubyte";
    std::string const setPathTestLabels = setPath + "/../Test_images/t10k-labels.idx1-ubyte";
    std::string const setPathResults = setPath + "/../Results_Network";

    NeuralNetwork network;

    /// Interpret if program is used for training or testing
    if (std::string(argv[1]) == "-train")
    {
        /// Determine path to store results:
        std::string setSavePath = setPathSave(setPathResults);

        /// Store file containing input arguments:
        std::ofstream outputFile;
        outputFile.open(setSavePath + "/Input_parameters");
        for (int iArgv = 2; iArgv < argc + 1; iArgv++)
        {
            outputFile << argv[iArgv] << "\t";
        }
        outputFile.close();

        // Cycle through arguments given and apply settings to the neural network
        for (int iArgc = 2; iArgc < argc; iArgc++)
        {
            if (std::string(argv[iArgc]) == "-layers")
            {
                /// Used to set the layers of the neural network.
                /// The first trailing argument should be the amount of layers. Subsequent the layer sizes are to be given in seperate arguments,
                /// starting from the input layer, up to the output layer. E.g. '-layers 3 {784,30,10}'
                int *pLayers = new int[atoi(argv[iArgc + 1])];
                std::cout << "Layers found: \n";
                for (int iLayer = 0; iLayer < atoi(argv[iArgc + 1]); iLayer++)
                {
                    pLayers[iLayer] = atoi(argv[iArgc + 2 + iLayer]);
                    std::cout << pLayers[iLayer] << "\t";
                }
                std::cout << "\n";
                network.initializeLayers(atoi(argv[iArgc + 1]), pLayers, setSavePath);
                delete[] pLayers;
                network.layerInfo();
                iArgc += atoi(argv[iArgc + 1]) + 1;
            }
            else if (std::string(argv[iArgc]) == "-param")
            {
                /// Used to set hyperparameters directly related to learning { samplesize before learning, eta (learning rate), lambda (regulatization)}
                network.setHyperParameters(atof(argv[iArgc + 1]), atof(argv[iArgc + 2]), atof(argv[iArgc + 3]));
                iArgc += 3;
            }
            else if (std::string(argv[iArgc]) == "-reduceLearning")
            {
                /// Use to reduce learning rate at given intervals. Parameter order: { reduction factor, after # cycles }
                network.setLearningReductionParameters(atof(argv[iArgc + 1]), atoi(argv[iArgc + 2]));
                iArgc += 2;
            }
            else
            {
                std::cerr << "The argument '" << argv[iArgc] << "' is unknown to the program. Use --help to show viable options." << std::endl;
                return 2;
            }
        }

        /// Load data for training:
        std::cout << "Loading data...\n";
        // Reads images and returns a matrix(pxValue, numOfImages)
        arma::dmat const trainingSet = readMnistImages(setPathTrainingImages);
        arma::uvec const trainingLabels = readMnistLabels(setPathTrainingLabels, trainingSet.n_cols);

        // Read test images to determine the score
        arma::dmat const testSet = readMnistImages(setPathTestImages);
        arma::uvec const testLabels = readMnistLabels(setPathTestLabels, testSet.n_cols);

        /// Start training:
        int iCountScore = 0;
        int iEpocheCount = 0;
        while (iEpocheCount < 70)
        {
            // Perform a training cycle (one epoche)
            network.training(trainingSet, trainingLabels);
            iEpocheCount += 1;

            std::cout << "Epoche counter: " << iEpocheCount << "\t\tAverage cost: " << arma::mean(network.cost) << "\n";
            iCountScore = network.computePerformance(testSet, testLabels);

            /// Store results every epoche
            network.storeResults();
        }
    }
    else if (std::string(argv[1]) == "-test")
    {
        /// Load test files
        std::cout << "Loading data...\n";
        arma::dmat const testSet = readMnistImages(setPathTestImages);
        arma::uvec const testLabels = readMnistLabels(setPathTestLabels, testSet.n_cols);

        /// Read parameters from parameter file
        std::ifstream inFile;
        std::string const setPathToLoad = setPathResults + "/Results_" + argv[2] + "/Input_parameters";

        inFile.open(setPathToLoad);
        if (inFile.is_open())
        {
            /// Read parameters to determine set correct network size
            int numOfLayers;
            int *pLayer;
            std::string arg;
            while (inFile >> arg)
            {
                if (arg == "-layers")
                {
                    inFile >> arg;
                    numOfLayers = stoi(arg);
                    pLayer = new int[numOfLayers];
                    for (int iLayer = 0; iLayer < numOfLayers; iLayer++)
                    {
                        inFile >> arg;
                        pLayer[iLayer] = stoi(arg);
                    }

                    /// Initialize weights and biases sizes and load results
                    network.initializeLayers(numOfLayers, pLayer, setPathResults + "/Results_" + argv[2]);
                    network.loadResults(setPathResults + "/Results_" + argv[2], numOfLayers, pLayer);
                }
            }
            /// Compute and output the score
            network.computePerformance(testSet, testLabels);
            inFile.close();
            delete[] pLayer;
        }
        else
        {
            std::cerr << "Unable to open a result file: " << setPathToLoad << std::endl;
            return 3;
        }

        // Cycle through arguments given and apply settings
        for (int iArgc = 3; iArgc < argc; iArgc++)
        {
            if (std::string(argv[iArgc]) == "-display")
            {
                /// Display results in random order
                arma::arma_rng::set_seed(std::chrono::high_resolution_clock::now().time_since_epoch().count());
                arma::uvec sequence = arma::randperm(testSet.n_cols);

                int digit;
                std::string strDigit;
                int countDisplays = 0;
                arma::Mat<double> imArma;
                for (arma::uword iSequence : sequence)
                {
                    /// Run a sample through the network and obtain result
                    digit = -1;
                    digit = network.feedForward(testSet.col(iSequence));
                    strDigit = std::to_string(digit);

                    /// Reshape the image vector into a matrix and convert to openCV format
                    imArma = reshape(round(testSet.col(iSequence) * 256), 28, 28);
                    cv::Mat imDigit(28, 28, CV_64FC1, imArma.memptr());

                    /// Display the sample image with the networks answer
                    displayImage(imDigit, strDigit);
                    countDisplays++;

                    /// Give option to end the program
                    if (cv::waitKey(3000) == 'q')
                    {
                        break;
                    };
                }
            }
            else
            {
                std::cerr << "The argument '" << argv[iArgc] << "' is unknown to the program. Use --help to show viable options." << std::endl;
                return 2;
            }
        }
    }
    else if (std::string(argv[1]) == "--help")
    {
        showUsage();
    }
    else
    {
        std::cerr << "The argument " << argv[1] << " is unknown to this program. Use --help to show viable options." << std::endl;
        return 2;
    }
    return 0;
}

ReadMNIST.h

arma::dmat readMnistImages( std::string);
arma::uvec readMnistLabels( std::string, arma::uword );
std::string getCurrentDir();

ReadMNIST.cpp

#include <armadillo>
#include <iostream>
#include <string>
#include "ReadMNIST.h"

#ifdef WINDOWS
#include <direct.h>
#define GetCurrentDir _getcwd
#else
#include <unistd.h>
#define GetCurrentDir getcwd
#endif

// Miscellaneous function
int reverseInt(int iSample)
{
    unsigned char ch1, ch2, ch3, ch4;
    ch1 = iSample & 255;
    ch2 = (iSample >> 8) & 255;
    ch3 = (iSample >> 16) & 255;
    ch4 = (iSample >> 24) & 255;
    return ((int)ch1 << 24) + ((int)ch2 << 16) + ((int)ch3 << 8) + ch4;
}

// Return a matrix containing the trainingset images. Format: (numOfImages, pxValue)
arma::dmat readMnistImages(std::string setPath)
{
    arma::umat imSet;
    std::ifstream file(setPath, std::ios::binary);
    if (file.is_open())
    {
        int magicNumber = 0;
        int numOfImages = 0;
        int imRows = 0;
        int imCols = 0;
        file.read((char *)&magicNumber, sizeof(magicNumber));
        magicNumber = reverseInt(magicNumber);
        file.read((char *)&numOfImages, sizeof(numOfImages));
        numOfImages = reverseInt(numOfImages);
        file.read((char *)&imRows, sizeof(imRows));
        imRows = reverseInt(imRows);
        file.read((char *)&imCols, sizeof(imCols));
        imCols = reverseInt(imCols);

        std::cout << "Images in the set: " << numOfImages << "\n";
        std::cout << "Image size: " << imRows << "*" << imCols << "\n";
        imSet.resize(numOfImages, imRows * imCols);

        for (int i = 0; i < numOfImages; ++i)
        {
            for (int r = 0; r < (imRows * imCols); ++r)
            {
                unsigned char input = 0;
                file.read((char *)&input, sizeof(input));
                imSet(i, r) = (double)input;
            }
        }
    }
    return (arma::conv_to<arma::dmat >::from(imSet.t())/256);
}

// Return a column containing the labels per image
arma::uvec readMnistLabels(std::string setPath, arma::uword numOfLabels)
{
    arma::uvec labelVector(numOfLabels);
    std::cout << "Number of labels: " << numOfLabels << "\n\n";

    std::ifstream file(setPath, std::ios::binary);
    if (file.is_open())
    {
        int magicNumber = 0;
        int numOfLabels = 0;
        file.read((char *)&magicNumber, sizeof(magicNumber));
        magicNumber = reverseInt(magicNumber);
        file.read((char *)&numOfLabels, sizeof(numOfLabels));
        numOfLabels = reverseInt(numOfLabels);

        for (int iSample = 0; iSample < numOfLabels; ++iSample)
        {
            unsigned char input = 0;
            file.read((char *)&input, sizeof(input));
            labelVector(iSample) = (double)input;
        }
    }
    return labelVector;
}

std::string getCurrentDir() {
   char buff[FILENAME_MAX]; //create string buffer to hold path
   GetCurrentDir( buff, FILENAME_MAX );
   std::string currentWorkingDir(buff);
   return currentWorkingDir;
}

Visualization.h

void displayImage( const cv::Mat &, const std::string );

Visualization.cpp

#include <opencv2/highgui.hpp>
#include <opencv2/imgproc.hpp>

void displayImage(const cv::Mat &im, const std::string strDigit)
{
    ////////////////////////////////////////////////////////////////////////////////////////////
    /// Scales the image into readable size and prints the network result onto image
    ////////////////////////////////////////////////////////////////////////////////////////////

    cv::Mat imScaled;
    cv::resize(im, imScaled, cv::Size(280, 280));

    // Write digit label on image
    cv::putText(imScaled,
                strDigit,
                cv::Point(5, 20),               // Coordinates
                cv::FONT_HERSHEY_COMPLEX_SMALL, // Font
                1.0,                            // Scale. 2.0 = 2x bigger
                cv::Scalar(255, 0, 0),          // BGR Color
                1);                             // Line Thickness (Optional)

    /// Write required action to close the program
    cv::putText(imScaled,
                "Press <q> to close",
                cv::Point(5, 275),              // Coordinates
                cv::FONT_HERSHEY_COMPLEX_SMALL, // Font
                0.5,                            // Scale. 2.0 = 2x bigger
                cv::Scalar(255, 0, 0),          // BGR Color
                1);                             // Line Thickness (Optional)

    cv::imshow("Test image", imScaled);
}
\$\endgroup\$
  • \$\begingroup\$ Are the header files complete, or have the top and bottom of those files been excluded from the review? \$\endgroup\$ – pacmaninbw Apr 23 at 17:24
  • \$\begingroup\$ The header files are complete as is. I did not remove any lines at the top nor bottom of the files. \$\endgroup\$ – rvdb Apr 30 at 7:09
3
\$\begingroup\$

This is all subjective:

But I would prefer if the constructor set up the NeuralNet and was ready to go. Once the object there is no need to call extra functions like initializeLayers() or setHyperParameters() or setLearningReductionParameters() these should all be things that are done as part of construction.

This implies that you need some form of config object (as these parameters seem complex) that you crete first that can be passed to the constructor of the NeuralNet. This config object could potentially be able to read and load its own state from a config file.

 NeuralNetConfig     config;
 // 1. Read default config from default config file.
 // 2. Add extra values to config from command line arguments.

 // When you are ready to go:
 NeuralNet           net(config);

This also covers one of your concenrs:

yet it is not responsible for reading the parameter file stating the network size to load


an object should be responsible for all of its affairs, including the in/output.

There are two types of objects.

  • One that handles business logic.
    In your case the logic of the NeuralNet.
  • Another type of object handlers resource management.
    In most cases this is memory management (this is things like std::vector).

Your object should fall into one of these two broad categories. If your object is mixing business logic and resource management then you need to consider why and can we easily split these apart.

In terms of Input/Output you can consider this part of the "business" logic or you can potentially delegate this to another class that understands this (its one of those grey areas).


However, where do I draw the line? For instance, the network is responsible for storing and loading of results

Yes (or maybe with the help of a delegate). But it should not be responsible for where it stores the results (this should be passed to the class). i.e. if you save the data to a file your class is not responsible for selecting or opening the file it will be passed an open file stream object onto which it can save the state.


Code Review:

//Smart pointers are used to ensure freeing of memory. The pointers are not always used and can therefore not be freed in a destructor
std::unique_ptr<int[]> sizeLayer;
std::unique_ptr<CLayer[]> pLayer;

Getting dangerously close to resource management. Also why are these not std::vector<> objects? Looking through the code these should definitely be std::vector<> objects.


Nice to put the parameters "names" in here. It helps in that little thing of "Self Documenting Code".

void initializeLayers(int, int *, std::string);
void setHyperParameters(double, double, double);
void layerInfo();
void training(const arma::dmat &, const arma::uvec &);
arma::uvec yVectorGenerator(const arma::uword &);
arma::dvec sigmoid(arma::dvec &);
arma::dvec Dsigmoid(arma::dvec &);
int computePerformance(const arma::dmat &, const arma::uvec &);
int feedForward(const arma::dvec &);
void setLearningReductionParameters(double, int);
void reduceLearnRate(double);
void storeResults();
void loadResults(const std::string &, int, int *);

Why are you passing a string by value here?

void initializeLayers(int, int *, std::string);

Why are you passing a pointer here?

void initializeLayers(int, int *, std::string);

Pointers are exceedingly rare in modern C++ (unless you are building some low level resource management object like a vector). The problem with pointers is that they do not convey ownership semantics and thus it is easy to leak or accidentally destroy something (ie. they are buggy to use).

When I look at the code that uses this I see it is very inefficient and dangerously written:

            int *pLayers = new int[atoi(argv[iArgc + 1])];

            // FILL IN pLayers
            network.initializeLayers(atoi(argv[iArgc + 1]), pLayers, setSavePath);
            delete[] pLayers;

The trouble is that you will leak that array if initializeLayers() throws an exception (and thus skips the delete). Inside the function you do the same thing (but at least assign it to smart pointer to prevent leaking).

// Allocate 
sizeLayer = std::unique_ptr<int[]>(new int[numOfLayers_]);
// And now copy.
for (int iLayer = 0; iLayer < numOfLayers_; iLayer++)
{
    sizeLayer[iLayer] = pLayerSize[iLayer];
}

By using vectors to do the resource management you can make your code a lot simpler and efficient.

            int countOfPlaters = atoi(argv[iArgc + 1]);
            std::vector<int> pLayers(countOfPlaters);
            // FILL IN pLayers
            
            network.initializeLayers(countOfPlaters, std::move(pLayers), setSavePath);

Now define the interface like this:

void initializeLayers(int, std::vector<int>&& players, std::string);

Inside the function to get a copy of the vector you simply do this:

 sizeLayer = std::move(players); // Note: sizeLayer is now declared std::vector<int>

This will effeciently move the data inside the vector without having to copy the whole vector. Memory is handeled even in exceptional cases and you have written less code.


If your method does not change the state of the object then it should be marked const. I am betting that this function does not change the state of the object.

void layerInfo();

Again using a pointer parameter.

void loadResults(const std::string &, int, int *);

Looking at the code this should be replaced by a std::vector.


Why are you doing this here?

    // Initialize: matrix and vector sizes
    pLayer[iLayer].a.set_size(sizeLayer[iLayer]);
    pLayer[iLayer].z.set_size(sizeLayer[iLayer]);
    pLayer[iLayer].b = arma::randn(sizeLayer[iLayer]);
    pLayer[iLayer].w.set_size(sizeLayer[iLayer], sizeLayer[iLayer - 1]);
    pLayer[iLayer].kD.set_size(sizeLayer[iLayer]);
    pLayer[iLayer].db = pLayer[iLayer].b;
    pLayer[iLayer].dw = pLayer[iLayer].w;

This should be in the constructor of CLayer.


This is a bad place to default_random_engine.

for (int iLayer = 1; iLayer < numOfLayers_; iLayer++)
{

    // STUFF

    std::default_random_engine generator{static_cast<long unsigned int>(std::chrono::high_resolution_clock::now().time_since_epoch().count())}; // Use high precision time to determine random seed

    std::normal_distribution<double> distribution(0.0, sqrt((double)sizeLayer[iLayer - 1]));            
}

The random number generators can be very expensive to initialize (as they can potentially hold a lot of state). You are supposed to initialize this once and re-use it as much as possible (then you would not need to use a high resolution timer to make it work). Just move this outside the loop and re-use for all your random numbers.

I would even move this into main and pass the random number generator as a parameter into the constructor to be re-used.

Note: During debugging it is useful to not have random numbers. To find a bug you may want to seed the generator with a known value so that you can set up the state as it was when you discovered a bug.

I always dump the seed into a log file. Also I allow the user to specify the seed at the command line so that I can reproduce an earlier run with the exact same input. Also for debugging this means having a single random number generator for the whole application makes things like debugging easier.


Also worth noting you can add a code review badge to your github readme.md file:

[![Code Review](http://www.zomis.net/codereview/shield/?qid=241074)](http://codereview.stackexchange.com/q/241074)

                           
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks for the review and the answer to my question. It is much appreciated! I am implementing the suggested method of using a config object and all other review comments. The code is not yet finished, but will be visible via the github link once its finished. \$\endgroup\$ – rvdb Apr 30 at 7:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.