Skip to content

Updates MobileNet Models to Support Graph Execution#227

Merged
dsmilkov merged 11 commits intotensorflow:masterfrom
tylerzhu-github:posenet-resnet-mobilenet-graph-mode-pr
Jun 8, 2019
Merged

Updates MobileNet Models to Support Graph Execution#227
dsmilkov merged 11 commits intotensorflow:masterfrom
tylerzhu-github:posenet-resnet-mobilenet-graph-mode-pr

Conversation

@tylerzhu-github
Copy link
Copy Markdown
Contributor

@tylerzhu-github tylerzhu-github commented May 31, 2019

This change is Reviewable

@tylerzhu-github tylerzhu-github changed the title Updates MobileNet Models to support Graph Execution Updates MobileNet Models to Support Graph Execution May 31, 2019
Copy link
Copy Markdown
Contributor

@dsmilkov dsmilkov left a comment

Choose a reason for hiding this comment

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

Reviewed 10 of 12 files at r1.
Reviewable status: 0 of 1 approvals obtained (waiting on @tylerzhu-github)


posenet/README.md, line 106 at r1 (raw file):

 * **multiplier** - Can be one of `1.01`, `1.0`, `0.75`, or `0.50` (The value is used *only* by the MobileNetV1 architecture and not by the ResNet architecture). It is the float multiplier for the depth (number of channels) for all convolution ops. The larger the value, the larger the size of the layers, and more accurate the model at the cost of speed. Set this to a smaller value to increase speed at the cost of accuracy.

 * **quantBytes** - This argument controls the bytes used for weight quantization (The value is *only* used by the ResNet50 model). The available options are:

Remove "The value is only used by Resnet50" since we support quantization for both mobilenet and resnet, right?


posenet/demos/camera.js, line 216 at r1 (raw file):

    updateGuiOutputStride(defaultMobileNetStride, [8, 16]);
    updateGuiMultiplier(defaultMobileNetMultiplier, [0.50, 0.75, 1.0])
    updateGuiQuantBytes(defaultMobileNetQuantBytes, [1, 2, 4]);

since both architectures now support the same input resolutions and quantBytes options, can we remove these lines here?


posenet/demos/coco.js, line 318 at r1 (raw file):

      updateGuiOutputStride([8, 16]);
      updateGuiMultiplier([0.5, 0.75, 1.0]);
      updateQuantBytes([4]);

[1, 2, 4] ? Also since mobilenet and resnet share the same options for inputRes and quant, do we have to enumerate it twice here?


posenet/demos/coco.js, line 333 at r1 (raw file):

    updateGuiOutputStride([8, 16]);
    updateGuiMultiplier([0.5, 0.75, 1.0]);
    updateQuantBytes([4]);

[1, 2, 4] ?


posenet/src/mobilenet.ts, line 22 at r1 (raw file):

export type OutputStride = 32|16|8;
export type MobileNetMultiplier = 0.25|0.50|0.75|1.0|1.01;

remove the 1.01


posenet/src/posenet_model.ts, line 388 at r1 (raw file):

  const quantBytes = config.quantBytes;
  const multiplier = config.multiplier;
  console.log('loadMobileNet');

remove log statement

Copy link
Copy Markdown
Contributor

@dsmilkov dsmilkov left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 approvals obtained (waiting on @tylerzhu-github)


posenet/src/checkpoints.ts, line 18 at r1 (raw file):

 */

const MOBILENET_BASE_URL = 'http://localhost:8080/';

make it the GCS url ending in tfjs-models/savedmodel/posenet/mobilenet/ (I'll upload it there)


posenet/src/checkpoints.ts, line 19 at r1 (raw file):

const MOBILENET_BASE_URL = 'http://localhost:8080/';
const RESNET50_BASE_URL = 'http://localhost:8080/';

this needs to be the GCS URL ending in tfjs-models/savedmodel/posenet/resnet50/ (I'll upload it there)

Copy link
Copy Markdown
Contributor Author

@tylerzhu-github tylerzhu-github left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 approvals obtained (waiting on @dsmilkov and @tylerzhu-github)


posenet/README.md, line 106 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

Remove "The value is only used by Resnet50" since we support quantization for both mobilenet and resnet, right?

Thanks Daniel, this is correct! I have updated the README.md


posenet/demos/camera.js, line 216 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

since both architectures now support the same input resolutions and quantBytes options, can we remove these lines here?

Great suggestions! Yes I did a refactor of the demo and the shared options are refactored.


posenet/demos/coco.js, line 318 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

[1, 2, 4] ? Also since mobilenet and resnet share the same options for inputRes and quant, do we have to enumerate it twice here?

Great suggestions! I did a refactoring and the shared options are refactored.


posenet/src/checkpoints.ts, line 18 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

make it the GCS url ending in tfjs-models/savedmodel/posenet/mobilenet/ (I'll upload it there)

Yes I have updated the URL.
I also sent you an email with the latest checkpoints. Is it possible to update the checkpoint files?


posenet/src/checkpoints.ts, line 19 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

this needs to be the GCS URL ending in tfjs-models/savedmodel/posenet/resnet50/ (I'll upload it there)

Yes I have updated the URL.
I also sent you an email with the latest checkpoints.
Is it possible to update the checkpoint files?

A minor request is that we can now remove the QxQ substring in the model.json since both ResNet and MoibleNet support arbitrary input resolution.


posenet/src/mobilenet.ts, line 22 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

remove the 1.01

done


posenet/src/posenet_model.ts, line 388 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

remove log statement

done

Copy link
Copy Markdown
Contributor Author

@tylerzhu-github tylerzhu-github left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 approvals obtained (waiting on @dsmilkov)


posenet/demos/coco.js, line 333 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

[1, 2, 4] ?

I refactored the GUI to remove duplication and all model uses default quantization options [1, 2, 4]

Copy link
Copy Markdown
Contributor

@dsmilkov dsmilkov left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 12 files at r1, 10 of 10 files at r2.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @tylerzhu-github)


posenet/src/checkpoints.ts, line 18 at r1 (raw file):

Previously, tylerzhu-github (tylerzhu-github) wrote…

Yes I have updated the URL.
I also sent you an email with the latest checkpoints. Is it possible to update the checkpoint files?

Will upload now, but this URL still says localhost:8080? (Maybe forgot to push your latest change?)


posenet/src/checkpoints.ts, line 19 at r1 (raw file):

Previously, tylerzhu-github (tylerzhu-github) wrote…

Yes I have updated the URL.
I also sent you an email with the latest checkpoints.
Is it possible to update the checkpoint files?

A minor request is that we can now remove the QxQ substring in the model.json since both ResNet and MoibleNet support arbitrary input resolution.

That's great. Will upload now. FYI, this still says localhost:8080

@tylerzhu-github
Copy link
Copy Markdown
Contributor Author

I have updated the checkpoint URLs. Feel free to merge it into the master branch and host a demo.

@dsmilkov
Copy link
Copy Markdown
Contributor

dsmilkov commented Jun 7, 2019

Thanks! Can you resolve the outstanding merge conflicts? After that, this is ready for merging! What an awesome cleanup. Thank you!

Copy link
Copy Markdown
Contributor

@dsmilkov dsmilkov left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained

@tylerzhu-github
Copy link
Copy Markdown
Contributor Author

Thanks Daniel for the reminder. I have resolved the merge conflict and the PR can be automatically merged.

Copy link
Copy Markdown
Contributor Author

@tylerzhu-github tylerzhu-github left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 1 of 1 approvals obtained


posenet/src/checkpoints.ts, line 18 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

Will upload now, but this URL still says localhost:8080? (Maybe forgot to push your latest change?)

this is fixed.


posenet/src/checkpoints.ts, line 19 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

That's great. Will upload now. FYI, this still says localhost:8080

It's fixed now

Copy link
Copy Markdown
Contributor

@dsmilkov dsmilkov left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 6 files at r4.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained

@dsmilkov dsmilkov merged commit 737a437 into tensorflow:master Jun 8, 2019
aptlin pushed a commit to aptlin/tfjs-models that referenced this pull request Jun 9, 2019
* upstream/master:
  Update Posenet to 2.0 (tensorflow#234)
  Updates MobileNet Models to Support Graph Execution (tensorflow#227)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants