Neural Networks in C

Question

This block of code is one of my first C header files I have made; it's ported from a Python program I made a few months ago for a project.

I was just looking for advice on how to increase the efficiency of the program. I tried to make everything as efficient as possible, but am I missing anything? Also, any suggestions to improve my programming in general are greatly appreciated!

/*
Neural networks library

By Ben Jones
1/13/2017
*/

#ifndef NEURAL_NETWORKS_INCLUDED
#define NEURAL_NETWORKS_INCLUDED

#include <stdlib.h>
#include <stdio.h>
#include <stdbool.h>
#include <math.h>
#include <time.h>

typedef struct neuron_struct neuron;

struct neuron_struct {
    float *weights;
    float *dw;
    float db;
    float neuron_error;
    unsigned int numinputs;
    unsigned int numoutputs;
    unsigned int inputindex;
    float lastout;
    float bias;
    float error;
    unsigned int *inpneurons;
    unsigned int *outneurons;
    float *inputs;
};

neuron *create_neuron(float *weights, unsigned int numweights){
    static neuron output;
    output.dw = malloc(sizeof(float)*numweights);
    output.weights = malloc(sizeof(float)*numweights);
    output.inputs = malloc(sizeof(float)*numweights);
    output.numinputs = numweights;
    int i;
    for(i = 0; i < numweights; i++){
        output.weights[i] = weights[i];
        output.dw[i] = 0;
    }
    return &output;
}

void free_neuron(neuron *n){
    free(n->dw);
    free(n->weights);
    free(n->inputs);
}

float neuron_eval(neuron *n, float *inputs){
    float sum = 0;
    int i;
    for(i = 0; i < n->numinputs; i++){
        sum += inputs[i]*n->weights[i];
        n->inputs[i] = inputs[i];
    }
    sum += n->bias;
    n->lastout = 1/(1+exp(-sum));
    return n->lastout;
}

void neuron_teach(neuron *n, float u, float m){
    int i;
    for(i = 0; i < n->numinputs; i++){
        n->dw[i] = n->inputs[i]*n->neuron_error*u + m*n->dw[i];
        n->weights[i] -= n->dw[i];
    }
    n->db = n->neuron_error*u + m*n->db;
    n->bias -= n->db;
}

void neuron_random_weights(neuron *n, float lower, float upper){
    float rand_flt;
    int i;
    for(i = 0; i < n->numinputs; i++){
        rand_flt = ((float) rand())/RAND_MAX;
        rand_flt *= upper-lower;
        rand_flt += lower;
        n->weights[i] = rand_flt;
    }
}

typedef struct layer_struct layer;

struct layer_struct{
    neuron *neurons;
    unsigned int numneurons;
    unsigned int numweights;
    float *outputs;
    float *errors;
};

layer *create_layer(float *weights, unsigned int numweights, unsigned int numneurons){
    static layer output;
    output.neurons = malloc(sizeof(neuron)*numneurons);
    int i;
    for(i = 0; i < numneurons; i++){
        output.neurons[i] = *create_neuron(weights, numweights);
    }
    output.numneurons = numneurons;
    output.numweights = numweights;
    output.outputs = malloc(sizeof(float)*numneurons);
    output.errors = malloc(sizeof(float)*numneurons);
    return &output;
}

void free_layer(layer *l){
    int i;
    for(i = 0; i < l->numneurons; i++){
        free_neuron(l->neurons+i);
    }
    free(l->neurons);
    free(l->outputs);
    free(l->errors);
}

void layer_eval(layer *l, float *inputs){
    int i;
    for(i = 0; i < l->numneurons; i++){
        l->outputs[i] = neuron_eval(l->neurons+i, inputs);
    }
}

void layer_teach(layer *l, float u, float m){
    int i;
    for(i = 0; i < l->numneurons; i++){
        neuron_teach(l->neurons+i, u, m);
    }
}

void layer_calc_errors(layer *l){
    int i;
    int j;
    float sum;
    for(i = 0; i < l->neurons[0].numinputs; i++){
        sum = 0;
        for(j = 0; j < l->numneurons; j++){
            sum += l->neurons[j].weights[i]*(l->neurons[j].neuron_error);
        }
        l->errors[i] = sum;
    }
}

void layer_random_weights(layer *l, float lower, float upper){
    int i;
    for(i = 0; i < l->numneurons; i++){
        neuron_random_weights(l->neurons+i, lower, upper);
    }
}

typedef struct feed_forward_struct feed_forward;

struct feed_forward_struct{
    layer *layers;
    unsigned int numlayers;
    unsigned int maxlayersize;
    float *outputs;
    float *inputs;
    float *errors;
};

feed_forward *create_feed_forward(unsigned int *layers, unsigned int numlayers){
    static feed_forward output;
    output.numlayers = numlayers;
    output.layers = malloc(sizeof(layer)*numlayers);
    output.maxlayersize = 0;
    int i;
    for(i = 0; i < numlayers; i++){
        if(layers[i] > output.maxlayersize){
            output.maxlayersize = layers[i];
        }
    }
    float *weights = malloc(sizeof(float)*output.maxlayersize);
    for(i = 0; i < output.maxlayersize; i++){
        weights[i] = 0;
    }
    float lastnumneurons = 1;
    for(i = 0; i < numlayers; i++){
        output.layers[i] = *create_layer(weights, lastnumneurons, layers[i]);
        lastnumneurons = layers[i];
    }
    free(weights);
    output.outputs = malloc(sizeof(float)*(output.layers[output.numlayers-1].numneurons));
    output.inputs = malloc(sizeof(float)*(output.layers[0].numneurons));
    output.errors = malloc(sizeof(float)*(output.layers[output.numlayers-1].numneurons));
    return &output;
}

void free_feed_forward(feed_forward *ff){
    int i;
    for(i = 0; i < ff->numlayers; i++){
        free_layer(ff->layers+i);
    }
    free(ff->layers);
    free(ff->outputs);
    free(ff->inputs);
    free(ff->errors);
}

void feed_forward_eval(feed_forward *ff, float *inputs){
    int i;
    for(i = 0; i < ff->layers[0].numneurons; i++){
        ff->inputs[i] = neuron_eval(ff->layers[0].neurons+i, inputs+i);
    }
    layer_eval(ff->layers+1, ff->inputs);
    if(ff->numlayers > 2){
        for(i = 2; i < ff->numlayers; i++){
            layer_eval(ff->layers+i, ff->layers[i-1].outputs);
        }
    }
    for(i = 0; i < ff->layers[ff->numlayers-1].numneurons; i++){
        ff->outputs[i] = ff->layers[ff->numlayers-1].outputs[i];
    }
}

void feed_forward_teach(feed_forward *ff, float *inputs, float *expected, float u, float m){
    feed_forward_eval(ff, inputs);
    int i;
    int j;
    for(i = 0; i < ff->layers[ff->numlayers-1].numneurons; i++){
        ff->errors[i] = ff->outputs[i] - expected[i];
    }
    for(i = ff->numlayers-1; i >= 0; i--){
        if(i == ff->numlayers-1){
            for(j = 0; j < ff->layers[i].numneurons; j++){
                ff->layers[i].neurons[j].neuron_error = ff->errors[j]*(ff->layers[i].neurons[j].lastout)*(1-ff->layers[i].neurons[j].lastout);
            }
        } else {
            for(j = 0; j < ff->layers[i].numneurons; j++){
                ff->layers[i].neurons[j].neuron_error = ff->layers[i+1].errors[j]*(ff->layers[i].neurons[j].lastout)*(1-ff->layers[i].neurons[j].lastout);
            }
        }
        layer_calc_errors(ff->layers+i);
    }
    for(i = 0; i < ff->numlayers; i++){
        layer_teach(ff->layers+i, u, m);
    }
}

void feed_forward_random_weights(feed_forward *ff, float lower, float upper){
    int i;
    for(i = 0; i < ff->numlayers; i++){
        layer_random_weights(ff->layers+i, lower, upper);
    }
}

typedef struct network_struct network;

struct network_struct{
    neuron *neurons;
    unsigned int numneurons;
    bool *nextupdates;
    unsigned int **net;
    unsigned int *numoutputs;
};

network *create_network(unsigned int **net, unsigned int *numoutputs, unsigned int numneurons){
    static network output;
    output = (network){.net = net, .numneurons = numneurons, .numoutputs = numoutputs, .neurons = malloc(sizeof(neuron)*numneurons), .nextupdates = malloc(sizeof(bool)*numneurons)};
    int i;
    int j;

    for(i = 0; i < numneurons; i++){
        output.neurons[i]=(neuron){.lastout = 0, .outneurons = malloc(sizeof(unsigned int)*numoutputs[i]), .numinputs = 0, .inputindex = 0};
        output.neurons[i].numoutputs = numoutputs[i];
        output.nextupdates[i] = false;
    }

    for(i = 0; i < numneurons; i++){
        for(j = 0; j < numoutputs[i]; j++){
            output.neurons[i].outneurons[j] = net[i][j];
            output.neurons[net[i][j]].numinputs += 1;
        }
    }

    for(i = 0; i < numneurons; i++){
        output.neurons[i].inpneurons = malloc(sizeof(unsigned int)*output.neurons[i].numinputs);
        output.neurons[i].weights = malloc(sizeof(float)*output.neurons[i].numinputs);
    }

    for(i = 0; i < numneurons; i++){
        for(j = 0; j < numoutputs[i]; j++){
            output.neurons[net[i][j]].inpneurons[output.neurons[net[i][j]].inputindex] = i;
        }
    }
    return &output;
}

void network_set_output(network *n, unsigned int neuron, float output){
    n->neurons[neuron].lastout = output;
    int i;
    for(i = 0; i < n->neurons[neuron].numoutputs; i++){
        n->nextupdates[n->neurons[neuron].outneurons[i]] = true;
    }
}

void network_step(network *n, unsigned int iterations){
    int i;
    int j;
    int k;
    for(i = 0; i < iterations; i++){
        static bool *new_nextupdates;
        new_nextupdates = malloc(sizeof(bool)*n->numneurons);

        for(j = 0; j < n->numneurons; j++){
            new_nextupdates[j] = false;
        }

        for(j = 0; j < n->numneurons; j++){
            if(n->nextupdates[j]){
                n->nextupdates[j] = false;
                float inps[n->neurons[j].numinputs];
                for(k = 0; k < n->neurons[j].numinputs; k++){
                    inps[k] = n->neurons[n->neurons[j].inpneurons[k]].lastout;
                }
                neuron_eval(&(n->neurons[j]), inps);
                for(k = 0; k < n->numoutputs[j]; k++){
                    new_nextupdates[n->neurons[j].outneurons[k]] = true;
                }
            }
        }
        free(n->nextupdates);
        n->nextupdates = new_nextupdates;
    }
}
#endif

Answer 1

I must say your code is really clean and easy to read. The naming is very good and coherent, you should consider answering questions here.

Now, enough for the compliments, now my suggestions:

in create_feed_forward you could use calloc which zeroes the memory way faster that you could do with a loop

float *weights = calloc(output.maxlayersize,sizeof(float));

in feed_forward_teach you'd make core more readable by declaring a pointer on ff->layers[i] and use that in the inner loop. Performance-wise, maybe the optimizer could do it, but it's more readable, and cannot be slower:

for(i = ff->numlayers-1; i >= 0; i--){
    layer *lay_i = ff->layers+i;
    if(i == ff->numlayers-1){
        for(j = 0; j < ff->layers[i].numneurons; j++){
            lay_i->neurons[j].neuron_error = ff->errors[j]*(lay_i->neurons[j].lastout)*(1-lay_i->neurons[j].lastout);
        }
    } else {
        for(j = 0; j < ff->layers[i].numneurons; j++){
            lay_i->neurons[j].neuron_error = ff->layers[i+1].errors[j]*(lay_i->neurons[j].lastout)*(1-lay_i->neurons[j].lastout);
        }
    }

in create_network you could initialize output.neuron[i] in one line instead of two:

    output.neurons[i]=(neuron){.lastout = 0, .outneurons = malloc(sizeof(unsigned int)*numoutputs[i]), .numinputs = 0, .inputindex = 0, .numoutputs = numoutputs[i]};

in network_step, you allocate the block for new_nextupdates, same remark as create_feed_forward: use calloc, and remove the static qualifier, which is useless.

    bool *new_nextupdates = calloc(n->numneurons,sizeof(bool));

Aside from that the code could benefit from some comments. There are currently none in your code.

Of course, use -O2 or -O3 optimization flags when compiling, and consider running your code through a profiler to identify the parts to hammer on in priority.

Answer 2

The code looks nice, but is see a few issues in it which I'll try to list here:

• This is supposedly a header file. However, you've included in here both declarations and definitions of your functions and structures. Normally, only the structures and the declarations of the functions should go into the header file, while the definitions of the functions should go into a source file (a .c). The way it is done at the moment, if you try to include your file into two different compilations units, you will have a problem at link time for multiple definitions of the functions...
• Normally, you define first your structures and typedef them after (or at the same time) but not before. Actually, I'm not sure the way you did it is compliant with the C standard (I could e wrong here).
• You have a potential major issue in the code with the way you allocate your structures: by using a static variable into the "constructors" and returning a pointer to it, you take the risk of storing references to the same structure over and over again. This could lead to tricky bugs and memory leaks. Fortunately, since in the calling side you dereference the pointer returned immediately, you actually store a copy of the structure. But then, why returning pointers in the first place? Just use a local structure for result and return it by value. Complexity-wise, this is the same for the code, but this is safer and cleaner.
• Performance-wise, it is hard to tell anything. Nothing strikes me as problematic, but the good way of doing is to profile your code and possibly optimize the hottest parts when identified.