wagenaartje / neataptic

:rocket: Blazing fast neuro-evolution & backpropagation for the browser and Node.js
https://wagenaartje.github.io/neataptic/
Other
1.19k stars 279 forks source link

70% faster activation of gated connections #89

Closed allbinmani closed 6 years ago

allbinmani commented 7 years ago

When looking at a performance graph in Chrome during training of an LSTM, I noticed the extensive use of indexOf in Node.activate(). I replaced this with a node index lookup object, which seems to have improved speed significantly. My specific training case went from ~43 seconds to ~13, an improvement of ~70%

wagenaartje commented 7 years ago

Thanks! Strange that I didn't implement it yet. I'll run some tests myself and then i'll merge it :+1:

allbinmani commented 7 years ago

Great! There are possibly a few more indexOf that could be optimized, but I'm not familiar enough with the code (yet ;) to take it on. Great work, keep it up!

allbinmani commented 7 years ago

Oh, I should mention that I ran my tests using TurboFan (--turbo flag to node, v8.2.1)

allbinmani commented 7 years ago

I took the time to optimize xtraces too, it seems to have improved things further, and tests still pass ;)

wagenaartje commented 7 years ago

You did a second, bigger update. I'm going to have to do lots of speed tests before I can pull this because it changes the foundation of the neural network architectures.

I also placed some comments: you replace the AND operator in every propagation function target check to the OR operator. I think this breaks propagation, I'll test this later. Targets are only received by output nodes!

You also used Python naming convention. Please use the Semi-standard JS rules for coding in this project. so now this_is_a_function but instead do thisIsAFunction (camelCase). Same holds for variables you created.

allbinmani commented 7 years ago

I fixed the naming of the new method in Node (but kept the leading _ to mark them 'internaI'), and the naming of the new xtrace key in Connection. Id be happy to contribute test cases if necessary, I did run and pass the included tests, but they might not quite cover everything?

wagenaartje commented 7 years ago

The Travis tests only test functionality and not speed sadly. Yes, that naming should be fine. The way to set up tests is to gather various data sets (XOR, NOT + some spatial data sets for LSMTs). Then run each of them a lot of times (1000+) and use 'performance.now()' to check time (for Chrome). With the same test we can examine the pre-PR speed and the after-PR speed.


About changing the logical statement. The way you change it would break the fundamental code of the networks. This will not be noticed by the Travis tests as those tests don't check for 'custom' networks. Imagine the following code:

const { Node, Group } = require('../dist/neataptic');

/** Create the network */
var A = new Group(2);
var B = new Group(4);
var C = new Node('output');

A.connect(B);
B.connect(C);

/** Dataset */
var trainingSet = [
  { input: [0,0], output: [0] },
  { input: [0,1], output: [1] },
  { input: [1,0], output: [1] },
  { input: [1,1], output: [0] }
];

/** Train the network */
var learningRate = .3;
var momentum = 0;

for (var i = 0; i < 1000; i++) {
  for (sample of trainingSet) {
    // when A activates 1
    A.activate(sample.input);
    B.activate();
    C.activate();

    C.propagate(learningRate, momentum, true, sample.output[0]);
    B.propagate(learningRate, momentum);
  }
}

// test it
for (var sample of trainingSet) {
  A.activate(sample.input);
  B.activate();

  console.log(sample.input, C.activate());
}

This trains a 2,3,1 network to learn the XOR data set. This is a custom set up architecture, without using any of the built in networks. This code will not work with the altered logical if statement. Why? Because nodes (or groups/layers of nodes) don't have a target 99% of the time. Propagation with a target is only necessary for output nodes. That's why it first chicks IF a target is present, and then checks if the target matches the requirements.

allbinmani commented 7 years ago

You are correct, my logic was flawed. Re-iterated, failed once, but got things back to working. However, it seems some test cases are not deterministic? It passes 'mostly' for me, but fails sometimes, with different errors (on master too);

1) Networks Learning capability GRU XOR
      GRU error
      + expected - actual

      -0.9
      +0.43632094074397804

      at Context.<anonymous> (test/network.js:335:14)
1) Networks Learning capability XNOR gate:

      AssertionError: expected 0.18339665218622025 to be below 0.002
      + expected - actual

      -0.18339665218622025
      +0.002

      at learnSet (test/network.js:52:10)
      at Context.<anonymous> (test/network.js:242:7)
wagenaartje commented 7 years ago

That is because networks are initialized randomly. There is a small chance it will fail, but just as you say 'mostly' it will pass.

XOR and XNOR are 'hard' non-linear data sets. So these fail sometimes. I have seen the GRU XOR fail rarely though. Most of the times the standalone test case fails.

But everything seems to be fine now. When I have time i'll make some speed tests and then i'll probably merge.

wagenaartje commented 7 years ago

I see you added performance test, have you managed to get results yet?

wagenaartje commented 6 years ago

I finally got time to check the performance tests, and I got as result that the current master branch is faster than the pull request.