Fixes for review comments, and to ensure that tensors are named as expected.

Author: Craigacp

tensorflow-core/tensorflow-core-api/src/main/java/org/tensorflow/ConcreteFunction.java

@@ -296,7 +296,7 @@ public Operand<?> call(Scope scope, Operand<?> argument) {
 
   @Override
   public Result call(Map<String, Tensor> arguments) {
-    // FIXME need to manage input/output operand lifetimes
+    // FIXME need to manage input operand lifetimes
     Ops tf = Ops.create();
     Map<String, Operand<?>> inputs = new LinkedHashMap<>(arguments.size());
 

tensorflow-core/tensorflow-core-api/src/main/java/org/tensorflow/Result.java

@@ -17,6 +17,7 @@
 */
 package org.tensorflow;
 
+import org.tensorflow.exceptions.TensorFlowException;
 import org.tensorflow.proto.framework.RunMetadata;
 
 import java.util.ArrayList;
@@ -27,6 +28,7 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 
 /**
@@ -35,15 +37,27 @@
  * <p>When this is closed it closes all the {@link Tensor}s inside it. If you maintain a
  * reference to a value after this object has been closed it will throw an {@link
  * IllegalStateException} upon access.
+ *
+ * <p>This class is not thread-safe with respect to the close operation. Multiple closers
+ * or one thread closing a tensor while another is reading may throw exceptions.
+ *
+ * <p>Note this class is used to manage the lifetimes of tensors produced by the
+ * TensorFlow runtime, from sessions and function calls. It is not used as an argument
+ * to {@code session.run} or function calls as users are in control of the creation
+ * of input tensors.
  */
 public final class Result implements AutoCloseable, Iterable<Map.Entry<String, Tensor>> {
     @Override
     public void close() {
         if (!closed) {
-            closed = true;
-            for (Tensor t : map.values()) {
-                t.close();
+            for (Tensor t : list) {
+                try {
+                    t.close();
+                } catch (TensorFlowException e) {
+                    logger.log(Level.WARNING, "Exception raised when closing tensor inside result.", e);
+                }
             }
+            closed = true;
         } else {
             logger.warning("Closing an already closed Result");
         }
@@ -111,12 +125,7 @@ public Tensor get(int index) {
      */
     public Optional<Tensor> get(String key) {
         if (!closed) {
-            Tensor value = map.get(key);
-            if (value != null) {
-                return Optional.of(value);
-            } else {
-                return Optional.empty();
-            }
+            return Optional.ofNullable(map.get(key));
         } else {
             throw new IllegalStateException("Result is closed");
         }
@@ -153,7 +162,10 @@ public Optional<RunMetadata> getMetadata() {
         }
 
         for (int i = 0; i < names.size(); i++) {
-            this.map.put(names.get(i), values.get(i));
+            Tensor old = this.map.put(names.get(i), values.get(i));
+            if (old != null) {
+                throw new IllegalArgumentException("Name collision in the result set, two outputs are named '" + names.get(i) + "'");
+            }
         }
         this.metadata = metadata;
         this.closed = false;

tensorflow-core/tensorflow-core-api/src/main/java/org/tensorflow/Session.java

@@ -308,7 +308,9 @@ public Runner feed(Operand<?> operand, Tensor t) {
      * @throws IllegalArgumentException if no output exists with the provided name
      */
     public Runner fetch(String operation) {
-      return fetch(graph.outputOrThrow(operation));
+      Runner r = fetch(graph.outputOrThrow(operation),false);
+      outputNames.add(operation);
+      return r;
     }
 
     /**
@@ -338,6 +340,20 @@ public Runner fetch(String operation, int index) {
      * @return this session runner
      */
     public Runner fetch(Output<?> output) {
+      return fetch(output, true);
+    }
+
+    /**
+     * Makes {@link #run()} return the Tensor referred to by {@code output}.
+     *
+     * <p>If {@code output} is a resource variable, will fetch the value.
+     *
+     * @param output the node to fetch the tensor from
+     * @param recordName Records the output name. If false the output name must be recorded by the
+     *                   calling method as otherwise the result object will throw on construction.
+     * @return this session runner
+     */
+    private Runner fetch(Output<?> output, boolean recordName) {
       if (output.env() != graph) {
         throw new IllegalStateException(
             "Can't fetch output "
@@ -380,6 +396,9 @@ public Runner fetch(Output<?> output) {
       } else {
         outputs.add(output);
       }
+      if (recordName) {
+        outputNames.add(output.name());
+      }
       return this;
     }
 
@@ -523,7 +542,6 @@ private Result runHelper(boolean wantMetadata) {
       TF_Operation[] outputOpHandles = new TF_Operation[outputs.size()];
       int[] outputOpIndices = new int[outputs.size()];
       TF_Operation[] targetOpHandles = new TF_Operation[targets.size()];
-      List<String> outputNames = new ArrayList<>();
 
       // It's okay to use Operation.getUnsafeNativeHandle() here since the safety depends on the
       // validity of the Graph and graphRef ensures that.
@@ -541,7 +559,6 @@ private Result runHelper(boolean wantMetadata) {
       for (Output<?> o : outputs) {
         outputOpHandles[idx] = (TF_Operation) o.getUnsafeNativeHandle();
         outputOpIndices[idx] = o.index();
-        outputNames.add(o.name());
         idx++;
       }
       idx = 0;
@@ -603,6 +620,7 @@ public void close() {
     private final ArrayList<Output<?>> inputs = new ArrayList<>();
     private final ArrayList<Tensor> inputTensors = new ArrayList<>();
     private final ArrayList<Output<?>> outputs = new ArrayList<>();
+    private final ArrayList<String> outputNames = new ArrayList<>();
     private final ArrayList<GraphOperation> targets = new ArrayList<>();
     private RunOptions runOptions = null;
   }

tensorflow-core/tensorflow-core-api/src/main/java/org/tensorflow/SessionFunction.java

@@ -112,6 +112,16 @@ public Result call(Map<String, Tensor> arguments) {
 
     signature.getOutputs().values().forEach(x -> runner.fetch(x.name));
 
-    return runner.run();
+    Result results = runner.run();
+
+    // Unpack the result object and rebuild it with the expected names.
+    LinkedHashMap<String, Tensor> outputs = new LinkedHashMap<>(results.size());
+    int i = 0;
+    for (String outputName : signature.outputNames()) {
+      outputs.put(outputName, results.get(i));
+      i++;
+    }
+
+    return new Result(outputs);
   }
 }

tensorflow-core/tensorflow-core-api/src/main/java/org/tensorflow/Tensor.java

@@ -210,7 +210,7 @@ static <T extends TType> T of(Class<T> type, Shape shape, ByteDataBuffer rawData
    * <p>When this methods retuns {@code true}, the tensor could be cast to a {@link SparseTensor
    * SparseTensor<T>} to access its <i>indices</i>, <i>values</i> and <i>denseShape</i> tensors.
    *
-   * @retrun true if this tensor is a sparse
+   * @return true if this tensor is a sparse
    */
   default boolean isSparse() {
     return false;