Skip to content

Conversation

@pyu10055
Copy link
Collaborator

@pyu10055 pyu10055 commented Aug 7, 2020

Add experiments flag to allow adding features that only work on the tf nightly build.
This also allows us to develop features for the changes on g3.

Also add transform_graph code that only works in g3, this allow us to sync up g3 changes to oss.

To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.


This change is Reviewable

@pyu10055 pyu10055 requested a review from lina128 August 7, 2020 04:36
Copy link
Collaborator

@lina128 lina128 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 (waiting on @lina128 and @pyu10055)


tfjs-converter/python/run-python-tests.sh, line 38 at r1 (raw file):

REQ_FILE="${SCRIPTS_DIR}/requirements-dev.txt"
if [[ $# > 1 ]]; then

The logic of this block depends on the number of arguments passed in, I wonder is that reliable, say in the future we add more arguments, this will need to change, right? Can we add some annotation to remind people also change this block if adding new argument. Or even better, can we use argparse here?


tfjs-converter/python/tensorflowjs/converters/tf_saved_model_conversion_v2.py, line 503 at r1 (raw file):

      frozen_graph, inputs, concrete_func.outputs)

  # Check if the TransformGraph is available to be imported,

Remove comma.


tfjs-converter/python/tensorflowjs/converters/tf_saved_model_conversion_v2.py, line 525 at r1 (raw file):

    def _strip_unused_nodes(frozen_graph, concrete_func, output_node_names):
      # Find the names of the input nodes needed to extract the minimal

Hi Ping, the minimal subgraph isn't very clear to me, from the code, it seems that raw inputs may not be needed, so the converted model shouldn't list them in the input signature. Is that right? If so, is it for a use case where a converted model needs the transformed inputs as inputs? Can you change this annotation to doccomment and also add explanation on what you mean by minimal subgraph and maybe provide the use case for this function? Thanks!

Copy link
Collaborator Author

@pyu10055 pyu10055 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 (waiting on @lina128 and @pyu10055)


tfjs-converter/python/run-python-tests.sh, line 38 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

The logic of this block depends on the number of arguments passed in, I wonder is that reliable, say in the future we add more arguments, this will need to change, right? Can we add some annotation to remind people also change this block if adding new argument. Or even better, can we use argparse here?

Added comments to the script, are you referring to python argparse, do you have example on how to use that in shell script?


tfjs-converter/python/tensorflowjs/converters/tf_saved_model_conversion_v2.py, line 525 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Hi Ping, the minimal subgraph isn't very clear to me, from the code, it seems that raw inputs may not be needed, so the converted model shouldn't list them in the input signature. Is that right? If so, is it for a use case where a converted model needs the transformed inputs as inputs? Can you change this annotation to doccomment and also add explanation on what you mean by minimal subgraph and maybe provide the use case for this function? Thanks!

This is to remove node in the graph that is not needed for the signature of the concrete function, basically the minimal graph that is needed for the input/putput pair

Copy link
Collaborator

@lina128 lina128 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 (waiting on @lina128 and @pyu10055)


tfjs-converter/python/run-python-tests.sh, line 38 at r1 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

Added comments to the script, are you referring to python argparse, do you have example on how to use that in shell script?

Sorry, my bad, I thought it's a python file.

@pyu10055 pyu10055 merged commit 178a54b into master Aug 7, 2020
@pyu10055 pyu10055 deleted the experiments_flag branch August 7, 2020 17:13
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