1st code review: rename 'scope_name' to 'prefix', etc.

7 files changed, 57 insertions, 43 deletions

diff --git a/tensorflow/c/c_api.cc b/tensorflow/c/c_api.cc
index 96653154e5..32b0b70620 100644
--- a/tensorflow/c/c_api.cc
+++ b/tensorflow/c/c_api.cc
@@ -2387,7 +2387,12 @@ void TF_FinishWhile(const TF_WhileParams* params, TF_Status* status,
 
 void TF_AbortWhile(const TF_WhileParams* params) { FreeWhileResources(params); }
 
-void TF_AddGradients(TF_Graph* g, const char* scope_name, TF_Output* y,
+void TF_AddGradients(TF_Graph* g, TF_Output* y, int ny, TF_Output* x, int nx,
+                     TF_Output* dx, TF_Status* status, TF_Output* dy) {
+  TF_AddGradientsWithPrefix(g, nullptr, y, ny, x, nx, dx, status, dy);
+}
+
+void TF_AddGradientsWithPrefix(TF_Graph* g, const char* prefix, TF_Output* y,
                      int ny, TF_Output* x, int nx, TF_Output* dx,
                      TF_Status* status, TF_Output* dy) {
 #ifdef __ANDROID__
@@ -2408,7 +2413,7 @@ void TF_AddGradients(TF_Graph* g, const char* scope_name, TF_Output* y,
 
     tensorflow::Scope scope =
         NewInternalScope(&g->graph, &status->status, &g->refiner)
-            .NewSubScope(scope_name != nullptr ? scope_name : "gradients");
+            .NewSubScope(prefix != nullptr ? prefix : "gradients");
 
     if (dx != nullptr) {
       std::vector<tensorflow::Output> dx_arg = OutputsFromTFOutputs(dx, ny);
diff --git a/tensorflow/c/c_api.h b/tensorflow/c/c_api.h
index e896f68ce0..8e49158957 100644
--- a/tensorflow/c/c_api.h
+++ b/tensorflow/c/c_api.h
@@ -1129,6 +1129,15 @@ TF_CAPI_EXPORT extern void TF_FinishWhile(const TF_WhileParams* params,
 // called after a successful TF_NewWhile() call.
 TF_CAPI_EXPORT extern void TF_AbortWhile(const TF_WhileParams* params);
 
+// Adds operations to compute the partial derivatives of sum of `y`s w.r.t `x`s.
+//
+// This method is the equivalent of calling TF_AddGradientsWithPrefix with a
+// nullptr prefix (which will create all gradients operations under "gradients/"
+// by default). See TF_AddGradientsWithPrefix for more details.
+TF_CAPI_EXPORT void TF_AddGradients(TF_Graph* g, TF_Output* y, int ny,
+                                    TF_Output* x, int nx, TF_Output* dx,
+                                    TF_Status* status, TF_Output* dy);
+
 // Adds operations to compute the partial derivatives of sum of `y`s w.r.t `x`s,
 // i.e., d(y_1 + y_2 + ...)/dx_1, d(y_1 + y_2 + ...)/dx_2...
 // `dx` are used as initial gradients (which represent the symbolic partial
@@ -1138,18 +1147,18 @@ TF_CAPI_EXPORT extern void TF_AbortWhile(const TF_WhileParams* params);
 // shapes in `y`.
 // The partial derivatives are returned in `dy`. `dy` should be allocated to
 // size `nx`.
-// `scope_name` names the scope (or sub-scope) into which all gradients
-// operations are added. If `scope_name` is nullptr, "gradients" is used by
-// default.
+// `prefix` names the scope into which all gradients operations are being added.
+// If `prefix` is nullptr, "gradients" is used by default.
 //
 // WARNING: This function does not yet support all the gradients that python
 // supports. See
 // https://www.tensorflow.org/code/tensorflow/cc/gradients/README.md
 // for instructions on how to add C++ more gradients.
-TF_CAPI_EXPORT void TF_AddGradients(TF_Graph* g, const char* scope_name,
-                                    TF_Output* y, int ny,
-                                    TF_Output* x, int nx, TF_Output* dx,
-                                    TF_Status* status, TF_Output* dy);
+TF_CAPI_EXPORT void TF_AddGradientsWithPrefix(TF_Graph* g, const char* prefix,
+                                              TF_Output* y, int ny,
+                                              TF_Output* x, int nx,
+                                              TF_Output* dx, TF_Status* status,
+                                              TF_Output* dy);
 
 // Create a TF_Function from a TF_Graph
 //
diff --git a/tensorflow/c/c_api_test.cc b/tensorflow/c/c_api_test.cc
index 2fe9e91583..adcdefbaf3 100644
--- a/tensorflow/c/c_api_test.cc
+++ b/tensorflow/c/c_api_test.cc
@@ -1475,16 +1475,16 @@ class CApiGradientsTest : public ::testing::Test {
   }
 
   void TestGradientsSuccess(bool grad_inputs_provided,
-      const char* scope_name = nullptr) {
+      const char* prefix = nullptr) {
     TF_Output inputs[2];
     TF_Output outputs[1];
     TF_Output grad_outputs[2];
     TF_Output expected_grad_outputs[2];
 
     BuildSuccessGraph(inputs, outputs);
-    BuildExpectedGraph(grad_inputs_provided, scope_name, expected_grad_outputs);
+    BuildExpectedGraph(grad_inputs_provided, prefix, expected_grad_outputs);
 
-    AddGradients(grad_inputs_provided, scope_name, inputs, 2, outputs, 1,
+    AddGradients(grad_inputs_provided, prefix, inputs, 2, outputs, 1,
                  grad_outputs);
 
     EXPECT_EQ(TF_OK, TF_GetCode(s_)) << TF_Message(s_);
@@ -1552,7 +1552,7 @@ class CApiGradientsTest : public ::testing::Test {
     EXPECT_EQ(*a_data, *b_data);
   }
 
-  void AddGradients(bool grad_inputs_provided, const char* scope_name,
+  void AddGradients(bool grad_inputs_provided, const char* prefix,
                     TF_Output* inputs, int ninputs, TF_Output* outputs,
                     int noutputs, TF_Output* grad_outputs) {
     if (grad_inputs_provided) {
@@ -1561,11 +1561,11 @@ class CApiGradientsTest : public ::testing::Test {
       TF_Operation* grad_inputs_op =
           FloatConst2x2(graph_, s_, grad_inputs_val, "GradInputs");
       grad_inputs[0] = TF_Output{grad_inputs_op, 0};
-      TF_AddGradients(graph_, scope_name, outputs, noutputs, inputs, ninputs,
-                      grad_inputs, s_, grad_outputs);
+      TF_AddGradientsWithPrefix(graph_, prefix, outputs, noutputs, inputs,
+                                ninputs, grad_inputs, s_, grad_outputs);
     } else {
-      TF_AddGradients(graph_, scope_name, outputs, noutputs, inputs, ninputs,
-                      nullptr, s_, grad_outputs);
+      TF_AddGradientsWithPrefix(graph_, prefix, outputs, noutputs, inputs,
+                                ninputs, nullptr, s_, grad_outputs);
     }
   }
 
@@ -1604,7 +1604,7 @@ class CApiGradientsTest : public ::testing::Test {
   }
 
   void BuildExpectedGraph(bool grad_inputs_provided,
-                          const char* grad_scope_name,
+                          const char* grad_prefix,
                           TF_Output* expected_grad_outputs) {
     // The expected graph looks like this if grad_inputs_provided.
     // If grad_inputs_provided is false, Const_0 will be a OnesLike op.
@@ -1633,9 +1633,9 @@ class CApiGradientsTest : public ::testing::Test {
     //
     const float const0_val[] = {1.0, 2.0, 3.0, 4.0};
     const float const1_val[] = {1.0, 0.0, 0.0, 1.0};
-    const char* grad_prefix = grad_scope_name;
-    if (grad_scope_name == nullptr) {
-      grad_prefix = "gradients";
+    const char* prefix = grad_prefix;
+    if (prefix == nullptr) {
+      prefix = "gradients";
     }
     TF_Operation* const0 =
         FloatConst2x2(expected_graph_, s_, const0_val, "Const_0");
@@ -1650,13 +1650,13 @@ class CApiGradientsTest : public ::testing::Test {
       const3 = FloatConst2x2(expected_graph_, s_, const3_val, "GradInputs");
     } else {
       const3 = OnesLike(expected_graph_, s_, matmul,
-          strings::StrCat(grad_prefix, "/OnesLike").c_str());
+          strings::StrCat(prefix, "/OnesLike").c_str());
     }
 
     TF_Operation* matmul1 = MatMul(expected_graph_, s_, const3, const1,
-        strings::StrCat(grad_prefix, "/MatMul").c_str(), false, true);
+        strings::StrCat(prefix, "/MatMul").c_str(), false, true);
     TF_Operation* matmul2 = MatMul(expected_graph_, s_, const0, const3,
-        strings::StrCat(grad_prefix, "/MatMul_1").c_str(), true, false);
+        strings::StrCat(prefix, "/MatMul_1").c_str(), true, false);
     expected_grad_outputs[0] = {matmul1, 0};
     expected_grad_outputs[1] = {matmul2, 0};
   }
@@ -1757,11 +1757,11 @@ TEST_F(CApiGradientsTest, MultipleCallsToAddGradients) {
 
   TF_Output outputs[1] = {{xy, 0}};
   TF_Output inputs[1] = {{x, 0}};
-  TF_AddGradients(graph_, nullptr, outputs, 1, inputs, 1, nullptr, s_, &dxy_dx);
+  TF_AddGradients(graph_, outputs, 1, inputs, 1, nullptr, s_, &dxy_dx);
   ASSERT_EQ(TF_OK, TF_GetCode(s_)) << TF_Message(s_);
 
   inputs[0] = {y, 0};
-  TF_AddGradients(graph_, nullptr, outputs, 1, inputs, 1, nullptr, s_, &dxy_dy);
+  TF_AddGradients(graph_, outputs, 1, inputs, 1, nullptr, s_, &dxy_dy);
   ASSERT_EQ(TF_OK, TF_GetCode(s_)) << TF_Message(s_);
 
   TF_SessionOptions* opts = TF_NewSessionOptions();
diff --git a/tensorflow/c/while_loop_test.cc b/tensorflow/c/while_loop_test.cc
index 12225fd1cb..d2d887f32c 100644
--- a/tensorflow/c/while_loop_test.cc
+++ b/tensorflow/c/while_loop_test.cc
@@ -431,8 +431,8 @@ TEST_F(CApiWhileLoopTest, Gradients) {
 
   // Create backprop graph
   TF_Output grad_output;
-  TF_AddGradients(graph_, nullptr, outputs_.data(), outputs_.size(),
-                  inputs_.data(), 1, nullptr, s_, &grad_output);
+  TF_AddGradients(graph_, outputs_.data(), outputs_.size(), inputs_.data(), 1,
+                  nullptr, s_, &grad_output);
   ASSERT_EQ(TF_OK, TF_GetCode(s_)) << TF_Message(s_);
 
   // Run gradient
diff --git a/tensorflow/java/src/main/java/org/tensorflow/Graph.java b/tensorflow/java/src/main/java/org/tensorflow/Graph.java
index f2bd3e99a5..353092701b 100644
--- a/tensorflow/java/src/main/java/org/tensorflow/Graph.java
+++ b/tensorflow/java/src/main/java/org/tensorflow/Graph.java
@@ -153,13 +153,14 @@ public final class Graph implements AutoCloseable {
    * If {@code dx} is null, the implementation will use dx of {@link org.tensorflow.op.core.OnesLike OnesLike} for all
    * shapes in {@code y}.
    * 
-   * @param scopeName name of the subscope into which gradients operations are added. If null, defaults to "gradients".
+   * @param prefix string prefix applied to names of nodes added to the graph to compute gradients.
+   *               If null, defaults to "gradients".
    * @param y output of the function to derive
    * @param x inputs of the function for which partial derivatives are computed
    * @param dx if not null, the partial derivatives of some loss function {@code L} w.r.t. {@code y}
    * @return the partial derivatives {@code dy} with the size of {@code x}
    */
-  public Output<?>[] addGradients(String scopeName, Output<?>[] y, Output<?>[] x, Output<?>[] dx) {
+  public Output<?>[] addGradients(String prefix, Output<?>[] y, Output<?>[] x, Output<?>[] dx) {
     Output<?>[] dy = new Output<?>[x.length];
     final long[] yHandles = new long[y.length];
     final int[] yIndices = new int[y.length];
@@ -191,7 +192,7 @@ public final class Graph implements AutoCloseable {
       // xHandles = [x0Handle, x1Handle, ...] and xIndices = [x0Index, x1Index, ..], we obtain 
       // dy = [dy0Handle, dy1Handle, ..., dy0Index, dy1Index, ...]
       long[] dyHandlesAndIndices =
-            addGradients(ref.nativeHandle(), scopeName, yHandles, yIndices, xHandles, xIndices, dxHandles, dxIndices);
+            addGradients(ref.nativeHandle(), prefix, yHandles, yIndices, xHandles, xIndices, dxHandles, dxIndices);
       int ndy = dyHandlesAndIndices.length >> 1;
       if (ndy != dy.length) {
         throw new IllegalStateException(String.valueOf(ndy) + " gradients were added to the graph when " + dy.length
@@ -210,7 +211,7 @@ public final class Graph implements AutoCloseable {
    * i.e., {@code dy/dx_1, dy/dx_2...}
    * <p> 
    * This is a simplified version of {@link #addGradients(Output[], Output[], Output[]) where {@code y} is
-   * a single output, {@code dx} is null and {@code scopeName} is null.
+   * a single output, {@code dx} is null and {@code prefix} is null.
    * 
    * @param y output of the function to derive
    * @param x inputs of the function for which partial derivatives are computed
@@ -331,7 +332,7 @@ public final class Graph implements AutoCloseable {
 
   private static native byte[] toGraphDef(long handle);
 
-  private static native long[] addGradients(long handle, String scopeName, long[] inputHandles, int[] inputIndices,
+  private static native long[] addGradients(long handle, String prefix, long[] inputHandles, int[] inputIndices,
       long[] outputHandles, int[] outputIndices, long[] gradInputHandles, int[] gradInputIndices);
 
   static {
diff --git a/tensorflow/java/src/main/java/org/tensorflow/op/Scope.java b/tensorflow/java/src/main/java/org/tensorflow/op/Scope.java
index 51a6ce8318..cf0b3d98c1 100644
--- a/tensorflow/java/src/main/java/org/tensorflow/op/Scope.java
+++ b/tensorflow/java/src/main/java/org/tensorflow/op/Scope.java
@@ -156,7 +156,7 @@ public final class Scope {
   }
   
   /**
-   * The name prefix of this scope
+   * The name prefix of this scope.
    * <p>
    * This value is the combination of the name of this scope and all of its parents, seperated by a '/', e.g.
    * <pre>{@code
diff --git a/tensorflow/java/src/main/native/graph_jni.cc b/tensorflow/java/src/main/native/graph_jni.cc
index a9b2ef6494..1bbda52641 100644
--- a/tensorflow/java/src/main/native/graph_jni.cc
+++ b/tensorflow/java/src/main/native/graph_jni.cc
@@ -135,7 +135,7 @@ Java_org_tensorflow_Graph_toGraphDef(JNIEnv* env, jclass clazz, jlong handle) {
 
 JNIEXPORT jlongArray JNICALL
 Java_org_tensorflow_Graph_addGradients(JNIEnv* env, jclass clazz, jlong handle,
-    jstring scope_name, jlongArray y_handles, jintArray y_indices,
+    jstring prefix, jlongArray y_handles, jintArray y_indices,
     jlongArray x_handles, jintArray x_indices,
     jlongArray dx_handles, jintArray dx_indices) {
 
@@ -163,16 +163,15 @@ Java_org_tensorflow_Graph_addGradients(JNIEnv* env, jclass clazz, jlong handle,
   }
   if (env->ExceptionCheck()) return nullptr;
 
-  jboolean is_copy;
-  const char* cscope_name = nullptr;
-  if (scope_name != nullptr) {
-    cscope_name = env->GetStringUTFChars(scope_name, &is_copy);
+  const char* cprefix = nullptr;
+  if (prefix != nullptr) {
+    cprefix = env->GetStringUTFChars(prefix, nullptr);
   }
   TF_Status* status = TF_NewStatus();
-  TF_AddGradients(g, cscope_name, y.get(), ny, x.get(), nx, dx.get(), status,
-      dy.get());
-  if (scope_name != nullptr) {
-    env->ReleaseStringUTFChars(scope_name, cscope_name);
+  TF_AddGradientsWithPrefix(g, cprefix, y.get(), ny, x.get(), nx, dx.get(),
+                            status, dy.get());
+  if (prefix != nullptr) {
+    env->ReleaseStringUTFChars(prefix, cprefix);
   }
   if (!throwExceptionIfNotOK(env, status)) {
     TF_DeleteStatus(status);