This adds thaw.js by freddyC · Pull Request #118 · BrainJS/brain.js

freddyC · 2018-01-11T15:51:02Z

should solve #82

Makes training async via thaw.js

Removes `reinforce` option

I felt this cleans up and simplifies the network without changing functionality.

Basically when you train if it doesn't have a set size for the network it initializes and uses your options (as normal) to dictate network size, layers and all

If you have already trained the network or if the network is loaded from JSON then you don't need to set it and it trains as normal.

Potential PROBLEMS

The train options training callback called with training stats test fails
a) I think this is a problem in thaw.js but haven't been able to track it down yet.
Should probably have more tests for the new train options
With training being async now this may create a new problem with trying to train when currently training. I haven't tested anything on that I was just thinking and about ramifications of this change. We might want to add some sort of (is training) variable and if someone tries to train while it's currently training either push this training to the end of current training or escape out with an error or something.

freddyC · 2018-01-11T15:57:22Z

-yarn.lock
+yarn.lock
+
+/dist**


yeah I shouldn't have done this

Can you remove the dist

robertleeplummerjr · 2018-01-13T03:08:19Z

Can we alter this to have have an async method, rather than changing the established method? If we do this than the sync method stays as it is, we get a new feature, don't break backward compatibility.

robertleeplummerjr · 2018-01-13T03:15:05Z

Interesting idea on reinforce, so if the net is defined, reinforce is implied?

freddyC · 2018-01-13T06:26:11Z

Yeah, that would make the default behavior allow us to pick up and improve training at any point.

I think this will lead naturally to reinforcement especially if the user can set a required confidence level, and then things that we are less confident on the user can have it set up to trigger him and he lets the net know the correct answer and that can be used to reinforce.

I will move the training to a train async method, Are you ok with keeping the reinforcement as I set it up or do you want me to revert that change?

robertleeplummerjr · 2018-01-13T14:45:23Z

We should ask the community on the thread where reinforce was added, but I think they'll love it.

freddyC · 2018-01-14T07:35:03Z

So I made the changes but am working on the tests, and I think I broke stuff so I will try and get it updated this weekend, just wanted to keep momentum on this.

### Makes training async via thaw.js ### Removes `reinforce` option I felt this cleans up and simplifies the network without changing functionality. Basically when you train if it doesn't have a set size for the network it initializes and uses your options (as normal) to dictate network size, layers and all If you have already trained the network or if the network is loaded from JSON then you don't need to set it and it trains as normal. ### PROBLEMS 1) The train options `training callback called with training stats` test fails - I think this is a problem in thaw.js but haven't been able to track it down yet. 2) Should probably have more tests for the new train options 3) With training being async now this may create a new problem with trying to train when currently training. I haven't tested anything on that I was just thinking and about ramifications of this change. We might want to add some sort of (is training) variable and if someone tries to train while it's currently training either push this training to the end of current training or escape out with an error or something.

robertleeplummerjr

Good job, had a few items to review. Ty ty!

robertleeplummerjr · 2018-01-13T22:56:20Z

-yarn.lock
+yarn.lock
+
+/dist**


Can you remove the dist

robertleeplummerjr · 2018-01-20T20:38:02Z

+   * @param cb
+   * @returns {{error: number, iterations: number}}
+   */
+  trainAsync (data, _options = {}, cb = function() {}) {


Can we just use a Promise here?

robertleeplummerjr · 2018-01-20T20:39:58Z

    };
+
+    if (this.sizes === null) {
+      this.initialize (data);


The space after a method I've not seen in javascript libraries, can we aim for justThis()

robertleeplummerjr · 2018-01-20T20:40:26Z

+   * @returns {{error: number, iterations: number}}
+   */
+  trainAsync (data, _options = {}, cb = function() {}) {
+    if (typeof _options === "function") {


We've been in the practice of using single quotes, would you be ok migrating to such?

robertleeplummerjr · 2018-01-20T20:41:18Z

 import likely from '../../dist/likely';

-describe('likely', () => {
+describe('likely', function () {


Lets keep the newer () => {} syntax where possible.

One thing I found out is that mocha doesn't play well with the new () => { syntax. I like it better but I was getting some really weird crashes that I think had to do with that. That being said I probably went overboard in changing it to function () {

Just use --compilers js:babel-core/register with mocha

…but sizes

perkyguy

What's the use case for the training time?

perkyguy · 2018-01-20T21:43:53Z

+    let errorThresh = options.errorThresh;
+    let log = options.log === true ? console.log : options.log;
+    let logPeriod = options.logPeriod;
+    let learningRate = _options.learningRate || this.learningRate || options.learningRate;


Why the triple places this could exist?

The only thing I can think of is that it gives preference to as so:

the passed in learning rate

the class's set learning rate

the default learning rate.

It is a little odd to me but I figured it was outside the scope of this pr

perkyguy · 2018-01-20T21:48:00Z

+          sum += this.trainPattern (data[i].input, data[i].output, learningRate);
+        }
+
+        res.error = sum / data.length;


Why not just res.error = data.reduce(...)/data.length?

I was talking to @robertleeplummerjr about it yesterday and we talked about the cost of closures. They aren't crazy expensive but there is a cost to them and I don't usually take them into consideration. I am open to either way but where it was simple to make the for loop and iterate I did that.

So I am open to either way :)

perkyguy · 2018-01-20T21:51:58Z

+
+        if (callback && (res.iterations % callbackPeriod === 0)) {
+          // JSON.parse/stringify to clone the object so the callback doesn't have side effects to training
+          callback(JSON.parse(JSON.stringify(res)));


Object.assign would be a little clearer for this?

👍 I didn't think of that but I will get it changed

perkyguy · 2018-01-20T21:59:37Z

+  data.forEach(d => {
+    var actual = net.run(d.input)
+    var expected = d.output;
+    assert.ok(actual < (expected + wiggle) && actual < (expected + wiggle), `failed to train "${op}" - expected: ${expected}, actual: ${actual}`);


Copy-pasta error? You have the same thing tested in this &&

Why not just do a .toFixed(1)?

Good idea on the .toFixed (1) and yeah that was meant to clamp that value.

perkyguy · 2018-01-20T21:59:54Z

+    data.forEach(d => {
+      var actual = net.run(d.input)
+      var expected = d.output;
+      assert.ok(actual < (expected + wiggle) && actual < (expected + wiggle), `failed to train "${op}" - expected: ${expected}, actual: ${actual}`);


Same thing tested?

perkyguy · 2018-01-20T22:11:52Z

+
+      if (callback && (res.iterations % callbackPeriod === 0)) {
+        // JSON.parse/stringify to clone the object so the callback doesn't have side effects to training
+        callback (JSON.parse (JSON.stringify (res)));


Object.assign?

or {...res}, which is essentially the same thing.

perkyguy · 2018-01-20T22:12:45Z

+    let outputSize = data[0].output.length;
+    let hiddenSizes = this.hiddenSizes;
+    if (!hiddenSizes) {
+      sizes.push(Math.max(3, Math.floor(inputSize / 2)));


If someone doesn't want hidden layers, the library will force it?

yep, that's how it is currently setup.

Why? Can @robertleeplummerjr she's some light on this?

Sorry, I read this as "yeah I did it this way, what of it?" Not as you probably intended it, "maintaining current functionality"

haha no worries, it's finding the balance between not changing too much but still changing things in a good way :)

Thats how it was when we got here, however one could do: { hiddenSizes: [] }

perkyguy · 2018-01-20T22:15:20Z

-      if (callback && (i % callbackPeriod === 0)) {
-        callback({ error: error, iterations: i });
+
+      if (callback && (res.iterations % callbackPeriod === 0)) {


The callback is for getting periodic training results? Not the finished trained model?

yeah that's how it is currently setup.

logging for the most part. Most people like to see training advance. I don't see this going away any time soon, as it was part of the model when the community took over.

Isn't that what the log option does? It's cool though, separate conversation for the community, this maintains current functionality

perkyguy · 2018-01-20T22:16:44Z

+          log(`iterations: ${res.iterations} training error: ${res.error}`);
+        }
+
+        if (callback && (res.iterations % callbackPeriod === 0)) {


Like above, the callback is for partially trained nets?

that is correct as well

perkyguy · 2018-01-20T22:19:11Z

+    const items = new Array(iterations);
+    const thaw  = new Thaw (items, {
+      delay: true,
+      each: () => {


This is the same as the standard train behavior, just delivered per iteration. This should be pulled out into its own function/method to keep it DRY

I agree, that'd be more ideal.

robertleeplummerjr · 2018-01-21T01:16:08Z

all on all I see some really cool things happening here.

Converts the async method to a promise, not a cb Updates tests Helps code be more dry

Adds back the () => in tests Ran `npm run make` to get things

freddyC · 2018-01-21T08:52:39Z

~~@robertleeplummerjr @perkyguy this I think is ready. The only thing I am not real happy with is the log tests. They pass but I don't know how to get them to log out correct.~~

this is what they look like right now. Mocha is logging something out and I don't know exactly what it's doing. Ideally I think we should be allowing the options to contain a function for log and call their function instead of just take a true or false and then using console.log. That would allow someone to plug in their custom logger. That I figured was outside the scope of this pr lol.

Simplified log test, and added in the ability to pass in custom log method. I thought this was outside of the scope of my pr but just kinda did it because the test output bugged me lol.

freddyC · 2018-01-22T04:56:58Z

I actually just added in the change for logging, it felt right, hopefully it is ok (a bit of scope creep but worse things have happened, I guarantee it).

robertleeplummerjr · 2018-01-22T17:27:11Z

   * @param {Number[]} sizes
   */
-  initialize(sizes) {
+  initialize (sizes) {


unneeded space

robertleeplummerjr · 2018-01-22T17:27:34Z

   * @returns {{error: number, iterations: number}}
   */
-  train(data, _options = {}) {
+  train (data, _options = {}) {


unneeded space

robertleeplummerjr · 2018-01-22T17:28:42Z

+   * @param data
+   * @returns sizes
+   */
+  _getSizesFromData (data) {


uneeded space

robertleeplummerjr · 2018-01-22T17:29:45Z

+   * if false passed in nothing is logged
+   * @returns error
+   */
+  _setLogMethod (log) {


unneeded space

robertleeplummerjr · 2018-01-22T17:29:55Z

+   * @param learning Rate
+   * @returns error
+   */
+  _calculateTrainingError (data, learningRate) {


unneeded space

robertleeplummerjr · 2018-01-22T17:30:02Z

+  _calculateTrainingError (data, learningRate) {
+    let sum = 0;
+    for (let i = 0; i < data.length; ++i) {
+      sum += this.trainPattern (data[i].input, data[i].output, learningRate);


unneeded space

robertleeplummerjr · 2018-01-22T17:30:11Z

+   * @param status { iterations: number, error: number}
+   * @param options
+   */
+  _checkTrainingTick (data, status, options) {


unneeded space

robertleeplummerjr · 2018-01-22T17:30:23Z

   */
-  train(data, _options = {}) {
-    const options = Object.assign({}, this.constructor.trainDefaults, _options);
+  train (data, _options = {}) {


unneeded space

robertleeplummerjr · 2018-01-22T17:31:07Z

-  train(data, _options = {}) {
-    const options = Object.assign({}, this.constructor.trainDefaults, _options);
+  train (data, _options = {}) {
+    let options = Object.assign({}, this.constructor.trainDefaults, _options);


options is never reassigned, use a const, not a let.

robertleeplummerjr · 2018-01-22T17:32:19Z

      callback: null,
      callbackPeriod: 10,
-      reinforce: false
+      trainTimeMs: -Infinity


Can the trainTimeMs be put into a seperate pr? Want to keep the pr's as lean as possible.

robertleeplummerjr · 2018-01-22T17:33:04Z

+   * @param cb
+   * @returns {{error: number, iterations: number}}
+   */
+  trainAsync (data, _options = {}) {


unneeded space, this line and next

robertleeplummerjr · 2018-01-22T17:33:23Z

+      };
+
+      if (this.sizes === null) {
+        let sizes = this._getSizesFromData (data);


unneeded space, this line and next.

robertleeplummerjr · 2018-01-22T17:33:35Z

-      }
-    }
+      const items = new Array(options.iterations);
+      const thaw  = new Thaw (items, {


unneeded space

robertleeplummerjr · 2018-01-22T17:33:46Z

+      const thaw  = new Thaw (items, {
+        delay: true,
+        each: () => {
+          this._checkTrainingTick (data, status, options);


unneeded space

robertleeplummerjr · 2018-01-22T17:34:18Z

   * @param learningRate
   */
-  trainPattern(input, target, learningRate) {
+  trainPattern (input, target, learningRate) {


unneeded space

robertleeplummerjr · 2018-01-22T17:34:24Z

   */
-  fromJSON(json) {
-    this.initialize(json.sizes);
+  fromJSON (json) {


unneeded space

still one space 😄

robertleeplummerjr · 2018-01-22T17:34:35Z


 let wiggle = 0.1;

+function isAround (actual, expected) {


unneeded space

robertleeplummerjr · 2018-01-22T17:34:42Z

 }

-describe('bitwise functions', () => {
+function testBitwiseAsync (data, op, done) {


unneeded space

robertleeplummerjr · 2018-01-22T17:52:07Z

+function testBitwiseAsync (data, op, done) {
+  let net = new brain.NeuralNetwork();
+  net
+    .trainAsync (data, { errorThresh: 0.003 })


there are several more of these, can you just handle all of them?

When my friend wanted me to give him a Charmander tattoo and he got this https://timedotcom.files.wordpress.com/2015/04/screen-shot-2015-04-17-at-11-12-39-am.png

`trainTimeMs` is gone and will be in a new pr `log` doesn't accept functions but will be in a new pr The tests pass (ugly but they pass lol) https://memegenerator.net/img/instances/75216624.jpg good thing no one actually looks at commit messages amirite?

robertleeplummerjr · 2018-01-26T13:42:59Z


 describe('hash input and output', () => {
-  it('runs correctly with array input and output', () => {
+  it ('runs correctly with array input and output', () => {


robertleeplummerjr · 2018-01-26T13:43:47Z

-      '#.....#' +
-      '###.##.'
-    ), net);
+      let result = likely(character(


spacing is odd here

robertleeplummerjr

This looks great. I had a couple suggestions, but they don't change functionality, it was just spacing. Ty for your hard work!

# Conflicts: # test/base/bitwise.js # test/base/json.js

robertleeplummerjr · 2018-01-27T13:13:47Z

ty so much for your hard work on this! Sorry about the conflicts, I went ahead and did a merge, tested, and merged.

freddyC · 2018-01-27T17:40:55Z

freddyC commented Jan 11, 2018

View reviewed changes

freddyC mentioned this pull request Jan 13, 2018

Stop training so that it may be continued later #88

Closed

freddyCfit added 2 commits January 20, 2018 13:24

updates training to train async or not and fixes the tests to conform

c5933bb

robertleeplummerjr requested changes Jan 20, 2018

View reviewed changes

Rebased onto master and small update for initialize to not send data …

7556c1c

…but sizes

freddyC force-pushed the add-thaw branch from f942c34 to 7556c1c Compare January 20, 2018 20:43

perkyguy reviewed Jan 20, 2018

View reviewed changes

Committing before I change to promise so I don't break things too bad

5e7a4e5

freddyCfit added 2 commits January 21, 2018 01:31

Updates all the things.

69cfeae

Converts the async method to a promise, not a cb Updates tests Helps code be more dry

Last things

a781ba8

Adds back the () => in tests Ran `npm run make` to get things

Loggity

6854435

Simplified log test, and added in the ability to pass in custom log method. I thought this was outside of the scope of my pr but just kinda did it because the test output bugged me lol.

robertleeplummerjr reviewed Jan 22, 2018

View reviewed changes

freddyCfit added 2 commits January 22, 2018 22:46

Remember that time

4782432

When my friend wanted me to give him a Charmander tattoo and he got this https://timedotcom.files.wordpress.com/2015/04/screen-shot-2015-04-17-at-11-12-39-am.png

Removed goodies

b4cd266

`trainTimeMs` is gone and will be in a new pr `log` doesn't accept functions but will be in a new pr The tests pass (ugly but they pass lol) https://memegenerator.net/img/instances/75216624.jpg good thing no one actually looks at commit messages amirite?

robertleeplummerjr reviewed Jan 26, 2018

View reviewed changes

robertleeplummerjr self-requested a review January 26, 2018 13:44

robertleeplummerjr approved these changes Jan 26, 2018

View reviewed changes

freddyC changed the base branch from master to develop January 26, 2018 21:21

Merge branch 'develop' into add-thaw

b828d84

# Conflicts: # test/base/bitwise.js # test/base/json.js

robertleeplummerjr merged commit f8aa38f into develop Jan 27, 2018

robertleeplummerjr deleted the add-thaw branch January 27, 2018 13:13

mubaidr mentioned this pull request Jul 24, 2018

Halt and Resume training #82

Closed

Uh oh!

Conversation

freddyC commented Jan 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Makes training async via thaw.js

Removes reinforce option

Potential PROBLEMS

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robertleeplummerjr commented Jan 13, 2018

Uh oh!

robertleeplummerjr commented Jan 13, 2018

Uh oh!

freddyC commented Jan 13, 2018

Uh oh!

robertleeplummerjr commented Jan 13, 2018

Uh oh!

freddyC commented Jan 14, 2018

Uh oh!

robertleeplummerjr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

perkyguy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

freddyC commented Jan 11, 2018 •

edited

Loading

Removes `reinforce` option

freddyC commented Jan 21, 2018 •

edited

Loading