Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
Create, save, load and run models using a new functional API #112
Conversation
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the |
|
@Shajan , I think you need to consent to the CLA, see message above, thank you |
|
@googlebot I consent. |
|
CLAs look good, thanks! |
|
I've noted a few small things, but overall it looks good. |
| runner.feed(t.getName(), tensor); | ||
| }); | ||
|
|
||
| Map<String, TensorInfo> outputToNode = signatureDef.getOutputsMap(); |
Craigacp
Sep 11, 2020
Collaborator
Are we guaranteed that the iteration order of the signature's outputs map is consistent?
Are we guaranteed that the iteration order of the signature's outputs map is consistent?
Shajan
Sep 13, 2020
•
Collaborator
Good observation, the ordering of signature's output map need not be consistent.
Here is why:
We depend on the ordering of output returned in the call List<Tensor<?>> resultTensors = runner.run(). Which depends on the order we pass in at runner.fetch(t.getName()), that happens to be the iteration order of signatureDef.getOutputsMap().
In runHelper, See creation of outputOpIndices based on order of outputs.add(output), which gets passed into Session.run(..)
Session.run uses outputOpIndices as the order of output, which is implemented in resolveOutputs and TF_Output
Good observation, the ordering of signature's output map need not be consistent.
Here is why:
We depend on the ordering of output returned in the call List<Tensor<?>> resultTensors = runner.run(). Which depends on the order we pass in at runner.fetch(t.getName()), that happens to be the iteration order of signatureDef.getOutputsMap().
In runHelper, See creation of outputOpIndices based on order of outputs.add(output), which gets passed into Session.run(..)
Session.run uses outputOpIndices as the order of output, which is implemented in resolveOutputs and TF_Output
Craigacp
Sep 14, 2020
Collaborator
So it will be consistent within an execution of call but could change across different VM executions if the signatureDef.getOutputsMap() is backed by a regular HashMap rather than a LinkedHashMap?
I ask because we have a tendency to iterate the value set of maps in demo code, and I don't want users to think those will be consistent if they aren't. Usually it's because there is only a single output, but if we're not maintaining ordering then people might be confused if they try to apply what we do in the demos/tests to larger problems.
So it will be consistent within an execution of call but could change across different VM executions if the signatureDef.getOutputsMap() is backed by a regular HashMap rather than a LinkedHashMap?
I ask because we have a tendency to iterate the value set of maps in demo code, and I don't want users to think those will be consistent if they aren't. Usually it's because there is only a single output, but if we're not maintaining ordering then people might be confused if they try to apply what we do in the demos/tests to larger problems.
karllessard
Sep 16, 2020
Author
Collaborator
The order we fetch the tensors does not really matter, as we don't return the outputs as a list but as a map, forcing the user to retrieve them by name instead of by ordinal position, like Session.Run does. And since this is not demo code neither, I think it is safe enough to leave it as is.
The order we fetch the tensors does not really matter, as we don't return the outputs as a list but as a map, forcing the user to retrieve them by name instead of by ordinal position, like Session.Run does. And since this is not demo code neither, I think it is safe enough to leave it as is.
...e/tensorflow-core-api/src/main/java/org/tensorflow/ConcreteFunction.java
Show resolved
Hide resolved
...e/tensorflow-core-api/src/main/java/org/tensorflow/SavedModelBundle.java
Outdated
Show resolved
Hide resolved
...e/tensorflow-core-api/src/main/java/org/tensorflow/SavedModelBundle.java
Outdated
Show resolved
Hide resolved
...e/tensorflow-core-api/src/main/java/org/tensorflow/SavedModelBundle.java
Show resolved
Hide resolved
...low-core/tensorflow-core-api/src/main/java/org/tensorflow/Signature.java
Show resolved
Hide resolved
...low-core/tensorflow-core-api/src/main/java/org/tensorflow/Signature.java
Outdated
Show resolved
Hide resolved
...nsorflow-core-api/src/test/java/org/tensorflow/ConcreteFunctionTest.java
Show resolved
Hide resolved
...nsorflow-core-api/src/test/java/org/tensorflow/ConcreteFunctionTest.java
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,45 @@ | |||
| Saved model created using Python @tf.function, tensorflow version 2.3.0. | |||
|
|
|||
| Python code used to create saved model below: | |||
Craigacp
Sep 11, 2020
Collaborator
Should we just check this in as a python file?
Should we just check this in as a python file?
karllessard
Sep 16, 2020
Author
Collaborator
@Shajan , what do you think? I think it could be a Python file, as long as we add a notice on top of that the file is only there for reference and is not being used anywhere in the code base.
@Shajan , what do you think? I think it could be a Python file, as long as we add a notice on top of that the file is only there for reference and is not being used anywhere in the code base.
karllessard
Sep 16, 2020
Author
Collaborator
I've moved forward and changed the file type to Python, please let me know if you think of any reason I should revert it to a text file.
I've moved forward and changed the file type to Python, please let me know if you think of any reason I should revert it to a text file.
Python models that contain tf.function is inconvenient to be consumed by Java clients. This proposal provides an API to (a) Invoke a tf.function, given the signature name (b) Retrieve the node name in the graph corresponding to a tf.function Co-authored-by: Shajan Dasan <sdasan@twitter.com> Save models as functions (#103) * Draft: Java API to use tf.function available on SavedModel. (#89) Python models that contain tf.function is inconvenient to be consumed by Java clients. This proposal provides an API to (a) Invoke a tf.function, given the signature name (b) Retrieve the node name in the graph corresponding to a tf.function Co-authored-by: Shajan Dasan <sdasan@twitter.com> * Change API for creating concrete functions and exporting them to a saved model Co-authored-by: Karl Lessard <klessard@gmail.com> Rename signature name to key Print function signature when converting to String Add method that returns the signature of all functions in a saved model Add unit tests for python created SavedModel with tf.function
| @@ -0,0 +1,48 @@ | |||
| # Saved model created using Python @tf.function, tensorflow version 2.3.0. | |||
Craigacp
Sep 17, 2020
Collaborator
This should have a copyright statement at the top.
This should have a copyright statement at the top.
|
LGTM |
| @@ -0,0 +1,45 @@ | |||
| Saved model created using Python @tf.function, tensorflow version 2.3.0. | |||
|
|
|||
| Python code used to create saved model below: | |||
Introducing a new
ConcreteFunctionclass, which allows you to create graphs, load/save models and run inference using the same functional API. See documentation in this PR for some examples.Exporting SavedModels only supports session-centric models, i.e. models that has a single main graph. This means that if multiple functions are saved with the model, they should all share the same graph for now. That limitation will be addressed later.
CC\ @Shajan