Simple Neural Network in Java

I had an assignment some weeks ago that consisted of making a simple McCulloch-Pitts neural network. I ended up coding it in a pretty OO style (or the OO style I've been taught), and I felt that my implementation might have been a bit excessive for being a one-off assignment: could this perhaps been implemented in a simpler, less verbose way? Possibly forsaking modularity and such since it's not really a concern. I had never seen any Neural Network implementations in Java beforehand, so I didn't really know what was a reasonable approach for this kind of thing.

This was implemented in Java - you were supposed to use C or C++, but I didn't know either of them so the teacher indulged me. It models a neural network with three input neurons, one output neuron and two layers of hidden neurons. The actual input-output specification is probably not important here, so I'll just explain the main class (NeuralNetwork.java); This class holds information about all neurons in the network and mediates all message passing between neurons: the neurons will in each fire one or more of their outgoing neurons in each timestep, based on the sum of the input of the ingoing neurons in the previous timestep. All of this message passing goes through the network: all the neurons don't know about each other or the network.

So the crux of the problem is simply to coordinate neurons and whether or not they fire their synapses in each timestep, and to calculate the responses of the neurons that they fire "towards" (through the synapses).

```
package main;

import java.util.HashMap;
import java.util.LinkedList;

public class NeuralNetwork {

//for debugging
int timestep = 0;

HashMap inputNeurons;
LinkedList internalNeurons;
LinkedList outputNeurons;

public NeuralNetwork
(HashMap inputNeurons, LinkedList internalNeurons, LinkedList outputNeurons)
{
this.inputNeurons = inputNeurons;
this.internalNeurons = internalNeurons;
this.outputNeurons = outputNeurons;
this.outputN

Since you didn't post the code of Neuron, InputNeuron and OutputNeuron so I just give some comments on the style.

Instance variables

private HashMap inputNeurons;
private LinkedList internalNeurons;
private LinkedList outputNeurons;

Instance variables should be kept private and expose via methods.

Ps: I'm not sure here if you intended to let them package-private for other classess (e.g Neuron, etc) to access them directly.

Prefer reference to objects by their interfaces

It is considered a good practice to reference to objects by their interfaces rather than concrete implementation (if they have one). Here you are using the Collection Framework so it'd be better if you declared the instance variables via interfaces.

private Map inputNeurons;
private List internalNeurons;
private List outputNeurons;

Make defensive copies

The constructor of NeuronNetwork uses directly the arguments passed to it which could lead to some subtle bugs. It'd be better if you make a copy of those arguments.

public NeuralNetwork(Map inputNeurons, List internalNeurons,
        List outputNeurons) {
    this.inputNeurons = new HashMap(inputNeurons);
    this.internalNeurons = new ArrayList(internalNeurons);
    this.outputNeurons = new ArrayList(outputNeurons);
    this.outputNeurons.get(0).getOutput(); // potential IndexOutOfBoundException
}

The last line in the constructor could cause IndexOutOfBoundException if the outputNeurons is empty. I'm not sure the purpose of this line.

Avoid using array as return type if possible

The method timeStep returns an int[] which is an array. IMO this is not good since array is less convenience and doesn't provides much useful behaviors. Returning a Collection or a List would be a better choice.

public Collection timestep(List inputs) {
    System.out.println("Network: timestep: " + timestep);

    for (String inp : inputs) {
        assert inputNeurons.get(inp) != null : "timestep was given a String as key for an InputNeuron that didn't exist";
        inputNeurons.get(inp).fire();
    }

    for (Neuron internal : internalNeurons) {
        internal.nextTimestep();
    }

    for (Neuron outputNeuron : outputNeurons) {
        outputNeuron.nextTimestep();
    }

    List output = new ArrayList(outputNeurons.size());
    for (OutputNeuron outputNeuron : outputNeurons) {
        output.add(outputNeuron.getOutput());
    }

    timestep++;
    resetAll();
    return output;
}

Misc

You could rename the method getNrTimestep() to getTimestep() for more elegant.

That's it. Hope these could help.

StackExchange Code Review Q#24218, answer score: 6