Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: add link how to convert model to tfjs #298

Closed
wants to merge 2 commits into from

Conversation

serv-inc
Copy link

No description provided.

@arnoegw arnoegw self-requested a review May 20, 2019 09:39
@arnoegw
Copy link
Contributor

arnoegw commented May 20, 2019

Hi @serv-inc, thanks for sending the pull request that references https://stackoverflow.com/a/56084489/1587329 from retrain.py.

If I understood correctly, it asks to use an older version of TFJS to do conversion from a frozen graph pb file. However, retrain.py is also capable of exporting to SavedModel. Can that be used?

@serv-inc
Copy link
Author

serv-inc commented May 22, 2019

Hi @arnoegw. If you know how to do this, feel welcome to respond to either tensorflow/tfjs#1576 or https://stackoverflow.com/questions/55849309/retrain-image-detection-with-mobilenet/56084489#56084489, I would gladly write it then. Otherwise, give me some time to check it out.

@serv-inc
Copy link
Author

Ok, it seems like the commands

python retrain.py --tfhub_module https://tfhub.dev/google/imagenet/mobilenet_v2_100_224/feature_vector/2 \
    --image_dir /tmp/flower_photos --saved_model_dir /tmp/saved_retrained_model
tensorflowjs_converter --input_format=tf_saved_model --output_format=tfjs_graph_model \
    --signature_name=serving_default --saved_model_tags=serve \
    /tmp/saved_retrained_model/ /tmp/converted_model/

produce a model.json file with current TFJS.

@arnoegw
Copy link
Contributor

arnoegw commented May 28, 2019

Thanks for confirming that it works with current TFJS via SavedModel! I thought it should, but I didn't really know. And thanks for posting it on StackOverflow as well.

With that clarification, what would you like to do with this pull request? There's some wisdom to be captured here, but I'm shy to add an official statement on TFJS support for retrain.py, because the TF Hub team seeks to replace it for TF2 by the much more modern tf2_image_retraining.ipynb. We don't want to keep looking after interoperability between retrain.py and TFJS.

How about adding general advice like:

For deployment to other environments, retrain.py can be called with flag --saved_model_dir to export the retrained model in SavedModel format.

?

@serv-inc
Copy link
Author

serv-inc commented May 28, 2019

With that clarification, what would you like to do with this pull request? There's some wisdom to be captured here, but I'm shy to add an official statement on TFJS support for retrain.py, because the TF Hub team seeks to replace it for TF2 by the much more modern tf2_image_retraining.ipynb. We don't want to keep looking after interoperability between retrain.py and TFJS.

Everyone would be happy if the hub-based retraining could be converted to TFJS. It seems like the saved model at least requires some tags for this.

How about adding general advice like:

For deployment to other environments, retrain.py can be called with flag --saved_model_dir to export the retrained model in SavedModel format.

That sounds very good. Would it be ok to add a link to SO for further information as well? Like:

For deployment to other environments, retrain.py can be called with flag --saved_model_dir to export the retrained model in SavedModel format. See also a related StackOverflow answer.

@serv-inc
Copy link
Author

I had not tested loading the models until now. Both ways of creating fail when loaded with

'className' and 'config' must set.
at new ValueError (/tmp/node_modules/@tensorflow/tfjs-layers/dist/errors.js:68:28)
at Object.deserializeKerasObject (/tmp/node_modules/@tensorflow/tfjs-layers/dist/utils/generic_utils.js:241:19)
at Object.deserialize (/tmp/node_modules/@tensorflow/tfjs-layers/dist/layers/serialization.js:29:28)
at /tmp/node_modules/@tensorflow/tfjs-layers/dist/models.js:290:45
at step (/tmp/node_modules/@tensorflow/tfjs-layers/dist/models.js:54:23)
at Object.next (/tmp/node_modules/@tensorflow/tfjs-layers/dist/models.js:35:53)
at fulfilled (/tmp/node_modules/@tensorflow/tfjs-layers/dist/models.js:26:58)

so this pull request should definitely not be merged as of now.

@serv-inc serv-inc closed this May 28, 2019
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