Update models to deeplearnjs 0.5.0

[HalfdanJ]

Making required changes to the knn image classifier and squeezenet models for 0.5.0. Added a simple demo to test knn image classifier.

---

This change is 

[HalfdanJ]

The travis errors seems to be because the demos and models are pulling in the old version of the code. Not sure how to get around that?

[nkreeger]

You'll have to update the demos I think (sorry). I think there are plans to move the demos and models out to make this easier in the future.

[dsmilkov]

This is awesome. Left a few comments (the only major comments are about not removing .scope, but replacing it with dl.tidy(), we renamed that).

Regarding Travis failure, you will have to update the versions in models/*/package.json and also the model versions in deps of demos/package.json to point to the newer models. Travis will still fail btw, since we haven't yet published these models. In the current build system, there is no way around that, but I can fix that right after we merge your PR - by publishing the new versions of those models on npm - which will make Travis happy

---

Reviewed 11 of 12 files at r1.
Review status: 11 of 12 files reviewed at latest revision, 17 unresolved discussions, some commit checks failed.

---

models/knn_image_classifier/demo.html, line 22 at r1 (raw file):

    // If dl isn't loaded, wait 1 second.
    if (dl == null) {
      await new Promise(resolve => setTimeout(resolve, 1000));

is this necessarily, or is this because we are being very cautious?

---

models/knn_image_classifier/demo.html, line 30 at r1 (raw file):

    resultElement.innerText = 'Training...';

    const catPixels = dl.Array3D.fromPixels(cat);

dl.fromPixels: doc here

---

models/knn_image_classifier/knn_image_classifier.ts, line 23 at r1 (raw file):

implements dl.Model

I think you can drop implements dl.Model (also dl.Model will change soon)

---

models/knn_image_classifier/knn_image_classifier.ts, line 34 at r1 (raw file):

  private varsLoaded = false;
  private squashLogitsDenominator = dl.Scalar.new(300);

dl.scalar(300)

---

models/knn_image_classifier/knn_image_classifier.ts, line 88 at r1 (raw file):

    this.clearTrainLogitsMatrix();

    this.math.scope((keep, track) => {

doc here
dl.tidy(() => {
...
dl.keep(this.classLogitsMatrices[classIndex]);
}

---

models/knn_image_classifier/knn_image_classifier.ts, line 131 at r1 (raw file):

    }

    return this.math.scope((keep) => {

same here: use dl.tidy(() => ... dl.keep());

---

models/knn_image_classifier/knn_image_classifier.ts, line 149 at r1 (raw file):

    const numExamples = this.getNumExamples();
    return dl

small suggestion: you can use chaining here:
a.matMul(b);

---

models/knn_image_classifier/knn_image_classifier.ts, line 237 at r1 (raw file):

    }
    if (ndarray1 == null) {
      return dl.clone(ndarray2);

chaining: ndarray2.clone()

---

models/knn_image_classifier/knn_image_classifier.ts, line 254 at r1 (raw file):

    const squashedVec = dl.div(vec, this.squashLogitsDenominator);

    const sqrtSum = dl.mulStrict(squashedVec, squashedVec).sum().sqrt();

you can chain all the way. Also we have .square(), so:
const sqrtSum = squashedVec.square().sum().sqrt();

---

models/knn_image_classifier/package.json, line 3 at r1 (raw file):

{
  "name": "deeplearn-knn-image-classifier",
  "version": "0.2.3",

update version

---

models/knn_image_classifier/package.json, line 12 at r1 (raw file):

  },
  "dependencies": {
    "deeplearn-squeezenet": "~0.1.9"

make it depend on the new, updated squeezenet version

---

models/squeezenet/package.json, line 3 at r1 (raw file):

{
  "name": "deeplearn-squeezenet",
  "version": "0.1.9",

update squeezenet version

---

models/squeezenet/squeezenet.ts, line 29 at r1 (raw file):

    'fire4'|'fire5'|'maxpool_3'|'fire6'|'fire7'|'fire8'|'fire9'|'conv10';

export class SqueezeNet implements dl.Model {

I think you can drop implements dl.Model (also dl.Model will change soon)

---

models/squeezenet/squeezenet.ts, line 31 at r1 (raw file):

export class SqueezeNet implements dl.Model {
  private variables: {[varName: string]: dl.Tensor};
  private preprocessOffset = Tensor1D.new([103.939, 116.779, 123.68]);

dl.tensor1d([...])

---

models/squeezenet/squeezenet.ts, line 66 at r1 (raw file):

  predictWithActivation(input: Array3D, activationName?: ActivationName):
      {logits: Array1D, activation: Array3D} {
    const [logits, activation] = this.math.scope(() => {

same here: use dl.tidy(() => ...);

---

models/squeezenet/squeezenet.ts, line 93 at r1 (raw file):

    }

    const pool2 = dl.maxPool(fire3, 3, 2, 'valid');

small suggestion here and elsewhere below: you can chain
fire3.maxPool(3, 2, 'valid');

---

models/squeezenet/squeezenet.ts, line 175 at r1 (raw file):

  async getTopKClasses(logits: Array1D, topK: number):
      Promise<{[className: string]: number}> {
    const predictions = this.math.scope(() => {

same here: use dl.tidy(() => ...);

---

Comments from Reviewable

[HalfdanJ]

They are updated, I can revert the demos to not be updated (since they are pulling the models from npm) and update them later. But knn-image-classifier is also pulling squeezenet from npm registry, and that will also result in errors.

[dsmilkov]

See my previous comment (above your comment). Update all versions as if everything was published to npm. Once we merge your PR (I'll force merge), I'll publish on npm and Travis will be happy

[HalfdanJ]

Review status: 8 of 13 files reviewed at latest revision, 17 unresolved discussions.

---

models/knn_image_classifier/demo.html, line 22 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

is this necessarily, or is this because we are being very cautious?

I copied it from the squeezenet demo, but don't know if it is

---

Comments from Reviewable

[HalfdanJ]

Review status: 7 of 14 files reviewed at latest revision, 17 unresolved discussions.

---

models/knn_image_classifier/demo.html, line 22 at r1 (raw file):

Previously, HalfdanJ (Jonas Jongejan) wrote…

I copied it from the squeezenet demo, but don't know if it is

Done.

---

models/knn_image_classifier/demo.html, line 30 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

dl.fromPixels: doc here

Done.

---

models/knn_image_classifier/knn_image_classifier.ts, line 23 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

implements dl.Model

I think you can drop implements dl.Model (also dl.Model will change soon)

Done.

---

models/knn_image_classifier/knn_image_classifier.ts, line 34 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

dl.scalar(300)

Done.

---

models/knn_image_classifier/knn_image_classifier.ts, line 88 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

doc here
dl.tidy(() => {
...
dl.keep(this.classLogitsMatrices[classIndex]);
}

Done.

---

models/knn_image_classifier/knn_image_classifier.ts, line 131 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

same here: use dl.tidy(() => ... dl.keep());

Done.

---

models/knn_image_classifier/knn_image_classifier.ts, line 149 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

small suggestion: you can use chaining here:
a.matMul(b);

Done.

---

models/knn_image_classifier/knn_image_classifier.ts, line 237 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

chaining: ndarray2.clone()

Done.

---

models/knn_image_classifier/knn_image_classifier.ts, line 254 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

you can chain all the way. Also we have .square(), so:
const sqrtSum = squashedVec.square().sum().sqrt();

Done.

---

models/knn_image_classifier/package.json, line 3 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

update version

Done.

---

models/knn_image_classifier/package.json, line 12 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

make it depend on the new, updated squeezenet version

Done.

---

models/squeezenet/package.json, line 3 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

update squeezenet version

Done.

---

models/squeezenet/squeezenet.ts, line 29 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

I think you can drop implements dl.Model (also dl.Model will change soon)

Done.

---

models/squeezenet/squeezenet.ts, line 31 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

dl.tensor1d([...])

Done.

---

models/squeezenet/squeezenet.ts, line 66 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

same here: use dl.tidy(() => ...);

Done.

---

models/squeezenet/squeezenet.ts, line 93 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

small suggestion here and elsewhere below: you can chain
fire3.maxPool(3, 2, 'valid');

Done.

---

models/squeezenet/squeezenet.ts, line 175 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

same here: use dl.tidy(() => ...);

Done.

---

Comments from Reviewable

[dsmilkov]

Nice work

---

Reviewed 1 of 12 files at r1, 7 of 7 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

models/knn_image_classifier/demo.html, line 22 at r1 (raw file):

Previously, HalfdanJ (Jonas Jongejan) wrote…

Done.

It's probably not necessary, but we can remove it later.

---

Comments from Reviewable