Skip to content
This repository was archived by the owner on Oct 17, 2021. It is now read-only.

Conversation

caisq
Copy link
Contributor

@caisq caisq commented May 14, 2018

FEATURE -- Add capability to save and tf.Models using the following mediums: browser local storage, IndexedDB, file downloads and updates and HTTP requests.

Towards: tensorflow/tfjs#13


This change is Reviewable

@caisq caisq requested review from nsthorat and bileschi May 14, 2018 20:22
Copy link
Contributor

@bileschi bileschi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. small comment question

`Cannot find any save handlers for URL '${handlerOrURL}'`);
} else if (handlers.length > 1) {
throw new ValueError(
`Found more then one (${handlers.length}) save handlers for ` +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this should be "than" not "then"

// Check the equality of the two models' weights.
const weights1 = model1.getWeights();
const weights2 = model2.getWeights();
// because the second layer has `useBias: false`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how I should be interpreting this comment

@caisq caisq changed the title Scheme-based Save and Load [DO NOT MERGE YET] Scheme-based Save and Load May 14, 2018
@nsthorat
Copy link

:lgtm_strong:


Review status: 0 of 3 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


src/engine/training.ts, line 1706 at r1 (raw file):

   * const saveResults = await model.save('indexeddb://my-model-1');
   *
   * const loadedModel = await tf.loadMOdel('indexeddb://my-model-1');

lower case o


src/engine/training.ts, line 1760 at r1 (raw file):

    const returnString = false;
    const modelConfig = this.toJSON(null, returnString);

name "null" here


Comments from Reviewable

add print outs to code snippets
@caisq
Copy link
Contributor Author

caisq commented May 15, 2018

Review status: 0 of 5 files reviewed at latest revision, 4 unresolved discussions, all commit checks successful.


src/model_save_test.ts, line 133 at r1 (raw file):

Previously, bileschi (Stanley Bileschi) wrote…

Not sure how I should be interpreting this comment

Done. This is cruft from an earlier iteration. Removed. Thanks for catching that.


src/engine/training.ts, line 1706 at r1 (raw file):

Previously, nsthorat (Nikhil Thorat) wrote…

lower case o

Done.


src/engine/training.ts, line 1745 at r1 (raw file):

Previously, bileschi (Stanley Bileschi) wrote…

nit: this should be "than" not "then"

Done.


Comments from Reviewable

@caisq
Copy link
Contributor Author

caisq commented May 15, 2018

Review status: 0 of 5 files reviewed at latest revision, 4 unresolved discussions, all commit checks successful.


src/engine/training.ts, line 1760 at r1 (raw file):

Previously, nsthorat (Nikhil Thorat) wrote…

name "null" here

Done.


Comments from Reviewable

@caisq caisq changed the title [DO NOT MERGE YET] Scheme-based Save and Load Scheme-based Save and Load May 15, 2018
@caisq caisq merged commit f32d05d into tensorflow:master May 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants