Stream Training Ability

[nickpoorman]

This pull request will give the network the ability to train using streams. Currently the only way to train is to supply an array of all the data. If your dataset is large enough you wont be able to load the entire thing into memory.

This pull request shouldn't break any of the current features or functionality. It only adds in the ability to train using streams.

The network is now a Write Stream that can be written to for each training iteration. It helps if you think of the network when training, as something that you are writing data to. ie. like pushing data into a brain.

[nickpoorman]

fixes #29

[harthur]

Thanks for this functionality! Can you add a mocha test to the test/unit directory too?

[timoxley]

+1. any updates on this?

[nickpoorman]

Ok. There's a mocha test in the test/unit directory now.

[harthur]

Looks like you define this transform twice. Here and above.

[harthur]

@nickpoorman thanks for taking the time to add a test. Sorry to get to it late, had a busy week. I haven't looked over all the code changes, as there's a lot of indentation change. but I've left a few comments on the readme changes.

I'm wondering what the advantage is to having a network itself be a write stream vs. the network just having a network.trainStream. On first think, the latter actually seems like it would be more self-explanatory when consuming the API. I think the former would make more sense if the network only had one function, but it does predicting as well as training. What were your thoughts?

[nickpoorman]

The additions in the readme are bit confusing, so I should probably get rid of them all together. They should really look at the test if for a more accurate way of using this anyway.

As far as the network being a WriteStream, I think it makes more sense that you can pipe directly to the network and emit events on it. Look at the stuff dominictarr puts out. If you really want to have network.trainStream then you are going to have to do some crazy binding calls throughout the implementation which I personally think will add more complexity to it.

[harthur]

Can you elaborate a bit on "crazy binding calls", give an example?

Holistically, I'd rather not have the whole network be a stream just for training. The network has bulk predicting as well, what if you wanted that to be streamed?

Additionally, the code net.write(data) isn't very explicit about what it's doing, you'd have to add a code comment saying that by writing to the network, you're writing to a training stream. If we wanted to keep the network as a stream, we'd want to add a more explicitly-named function that wraps this.

[mikeumus]

👍 and watching. 8)

[nickpoorman]

Ok, I pulled the stream stuff into its own TrainStream class that can manipulate the network. The tests and README were also updated.

nick : brain $ mocha test/unit/*

  ․․․․․․․․․․․․․․․․․․․․․․․․․․․

  27 passing (866 ms)

[harthur]

Changing from NN.prototype = to NN.prototype.funcname = ... isn't a change I want to make in this patch. If only because this makes the diff busy and it's really hard to tell what the real changes are in this PR. Also this is a style decision I made for this library. People use the prototype.func pattern, but that wasn't my choice for this library.

[nickpoorman]

Changes should be easy to see now:
https://github.com/harthur/brain/pull/30/files

[harthur]

Thanks a bunch for doing that. I'll take a look at this very soon.

[TeRq]

Waiting for it.

[harthur]

Maybe we should just throw here then, unless you had a reason for null? Also, the rest of the file's style applied to this would look like:

if (!opts.neuralNetwork) {
  return null;
}

[harthur]

This looks great, thanks Nick. I have just a couple comments.

[nickpoorman]

Ok. I fixed the comments.

[harthur]

Squashed and merged.

Thanks again for this. I appreciate the hard work and it looks like several other people do too.

[mikeumus]

Woo hoo!

Thank you @harthur for the detailed merge.
and thank you @nickpoorman for the pull-req! 👍

[TeRq]

Jpiiii!