Change completeSubgraph signature, leave filtering to users

Signed-off-by: Ryan Nett <[email protected]>

Author: rnett

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

@@ -220,52 +220,16 @@ private GraphOperation graphOp(Operand<?> operand) {
   }
 
   /**
-   * Finds the operations used to produce {@code outputs} from {@code inputs}, or throws if that is not possible.
-   * Includes control dependencies.
-   *
-   * @param inputs the inputs of the subgraph.  Must be from single output ops.  May not be null.
-   * @param outputs the outputs of the subgraph.  May not be null.
-   * @param allowNoInputBodyOps whether to allow 0-input ops in the body.  For more specificy use {@link
-   * #completeSubgraph(Set, Set, Set, Set)}.
-   * @return the set of operations needed to calculate outputs from inputs, including outputs and inputs
-   * @throws IllegalStateException if outputs depends on ops outside of the subgraph (i.e. is not calculable based
-   * solely on inputs)
-   * @see #completeSubgraph(Set, Set, Set, Set)
-   */
-  public Set<GraphOperation> completeSubgraph(Set<Operand<?>> inputs, Set<Operand<?>> outputs,
-      boolean allowNoInputBodyOps) {
-    return completeSubgraph(inputs, outputs, null, allowNoInputBodyOps ? Collections.emptySet() : null);
-  }
-
-  /**
-   * Finds the operations used to produce {@code outputs} from {@code inputs}, or throws if that is not possible.
-   * Includes control dependencies.
-   *
-   * If both {@code allowedNoInputBodyOps} and {@code forbiddenNoInputBodyOps} are {@code null}, forbids 0-input ops in
-   * the body. To allow all ops in the body, use {@code null} for {@code allowedNoInputBodyOps} and the empty set for
-   * {@code forbiddenNoInputBodyOps}.
+   * Finds the operations used to produce {@code outputs}, assuming {@code inputs} are provided. Includes control dependencies.
+   * <p>
+   * Note that this function can easily return ops upstream of inputs as part of the body.  Depending on your use, the
+   * returned body should probably be filtered for {@code Placeholder}s, at least.
    *
    * @param inputs the inputs of the subgraph.  Must be from single output ops.  May not be null.
    * @param outputs the outputs of the subgraph.  May not be null.
-   * @param allowedNoInputBodyOps types of ops to allow as 0-input ops in the body.  Allows all (except {@code
-   * forbiddenNoInputBodyOps}) if null.
-   * @param forbiddenNoInputBodyOps types of ops to never allow as 0-input ops in the body.  Forbids all (except {@code
-   * allowedNoInputBodyOps}) if null.
    * @return the set of operations needed to calculate outputs from inputs, including outputs and inputs
-   * @throws IllegalStateException if outputs depends on ops outside of the subgraph (i.e. is not calculable based
-   * solely on inputs)
-   * @see #completeSubgraph(Set, Set, boolean)
    */
-  public synchronized Set<GraphOperation> completeSubgraph(Set<Operand<?>> inputs, Set<Operand<?>> outputs,
-      Set<String> allowedNoInputBodyOps, Set<String> forbiddenNoInputBodyOps) {
-
-    if (forbiddenNoInputBodyOps != null && allowedNoInputBodyOps != null) {
-      for (String t : forbiddenNoInputBodyOps) {
-        if (allowedNoInputBodyOps.contains(t)) {
-          throw new IllegalArgumentException("Can't allow and forbid op type " + t + ".");
-        }
-      }
-    }
+  public synchronized Set<GraphOperation> completeSubgraph(Set<Operand<?>> inputs, Set<Operand<?>> outputs) {
 
     if (inputs == null) {
       throw new IllegalArgumentException("Inputs can't be null.");
@@ -301,18 +265,6 @@ public synchronized Set<GraphOperation> completeSubgraph(Set<Operand<?>> inputs,
         continue;
       }
 
-      if (op.numControlInputs() + op.numInputs() == 0) {
-        // inverted: (nothing is forbidden || not forbidden) and (everything is allowed || allowed) and (not both null)
-        if ((forbiddenNoInputBodyOps != null && forbiddenNoInputBodyOps.contains(op.type()))
-            || (allowedNoInputBodyOps != null && !allowedNoInputBodyOps.contains(op.type()))
-            || (forbiddenNoInputBodyOps == null && allowedNoInputBodyOps == null)) {
-          throw new IllegalStateException("Operation " + op
-              + " of type " + op.type() +
-              " has no inputs and is not an allowed 0-input op type, but is not set as an input.  "
-              + "It is impossible to calculate the specified outputs with the given inputs.");
-        }
-      }
-
       for (GraphOperation control : op.controlInputs()) {
         if (!inputOps.contains(control)) {
           currents.add(control);

tensorflow-core/tensorflow-core-api/src/test/java/org/tensorflow/GraphTest.java

@@ -131,8 +131,7 @@ public void completeSubgraph() {
       Operand<TInt32> output = tf.math.mul(d, c);
 
       Set<GraphOperation> subgraph = g
-          .completeSubgraph(new LinkedHashSet<>(Arrays.asList(control, a, b, c)), Collections.singleton(output), null,
-              null);
+          .completeSubgraph(new LinkedHashSet<>(Arrays.asList(control, a, b, c)), Collections.singleton(output));
 
       assertEquals(new LinkedHashSet<>(Arrays.asList(control.op(), a.op(), b.op(), c.op(), d.op(), output.op())),
           subgraph);
@@ -152,56 +151,13 @@ public void completeSubgraphWithConstants() {
       Operand<TInt32> output = tf.math.mul(d, c);
 
       Set<GraphOperation> subgraph = g
-          .completeSubgraph(Collections.emptySet(), Collections.singleton(output), Collections.singleton(
-              Constant.OP_NAME), null);
+          .completeSubgraph(Collections.emptySet(), Collections.singleton(output));
 
       assertEquals(new LinkedHashSet<>(Arrays.asList(control.op(), a.op(), b.op(), c.op(), d.op(), output.op())),
           subgraph);
     }
   }
 
-  @Test
-  public void completeSubgraphMissingInput() {
-    try (Graph g = new Graph()) {
-      Ops tf = Ops.create(g);
-      Operand<TInt32> control = tf.constant(0);
-      Operand<TInt32> a = tf.withControlDependencies(Collections.singletonList(control)).constant(1);
-      Operand<TInt32> b = tf.constant(2);
-      Operand<TInt32> c = tf.constant(3);
-
-      Operand<TInt32> d = tf.math.add(a, b);
-      Operand<TInt32> output = tf.math.mul(d, c);
-
-      try {
-        g.completeSubgraph(new LinkedHashSet<>(Arrays.asList(control, b)), Collections.singleton(output), null, null);
-        fail();
-      } catch (IllegalStateException e) {
-        assertTrue(e.getMessage().contains("is not set as an input"));
-      }
-    }
-  }
-
-  @Test
-  public void completeSubgraphMissingControlInput() {
-    try (Graph g = new Graph()) {
-      Ops tf = Ops.create(g);
-      Operand<TInt32> control = tf.constant(0);
-      Operand<TInt32> a = tf.withControlDependencies(Collections.singletonList(control)).constant(1);
-      Operand<TInt32> b = tf.constant(2);
-      Operand<TInt32> c = tf.constant(3);
-
-      Operand<TInt32> d = tf.math.add(a, b);
-      Operand<TInt32> output = tf.math.mul(d, c);
-
-      try {
-        g.completeSubgraph(new LinkedHashSet<>(Arrays.asList(a, b)), Collections.singleton(output), null, null);
-        fail();
-      } catch (IllegalStateException e) {
-        assertTrue(e.getMessage().contains("is not set as an input"));
-      }
-    }
-  }
-
   @Test
   public void failImportOnInvalidGraphDefs() {
     try (Graph g = new Graph()) {