Branch 167933991

WORKSPACE

@@ -27,7 +27,6 @@ load("//tensorflow:workspace.bzl", "tf_workspace")
 #    path = "<PATH_TO_SDK>",
 #)
 #
-# Android NDK r12b is recommended (higher may cause issues with Bazel)
 #android_ndk_repository(
 #    name="androidndk",
 #    path="<PATH_TO_NDK>",

tensorflow/c/c_test_util.cc

@@ -27,6 +27,10 @@ static void Int32Deallocator(void* data, size_t, void* arg) {
   delete[] static_cast<int32_t*>(data);
 }
 
+static void DoubleDeallocator(void* data, size_t, void* arg) {
+  delete[] static_cast<double*>(data);
+}
+
 TF_Tensor* Int8Tensor(const int64_t* dims, int num_dims, const char* values) {
   int64_t num_values = 1;
   for (int i = 0; i < num_dims; ++i) {
@@ -63,6 +67,14 @@ TF_Tensor* Int32Tensor(int32_t v) {
                       &Int32Deallocator, nullptr);
 }
 
+TF_Tensor* DoubleTensor(double v) {
+  const int num_bytes = sizeof(double);
+  double* values = new double[1];
+  values[0] = v;
+  return TF_NewTensor(TF_DOUBLE, nullptr, 0, values, num_bytes,
+                      &DoubleDeallocator, nullptr);
+}
+
 // All the *Helper methods are used as a workaround for the restrictions that
 // one cannot call ASSERT_* methods in non-void-returning functions (when
 // exceptions are disabled during compilation)
@@ -105,6 +117,12 @@ TF_Operation* ScalarConst(int32_t v, TF_Graph* graph, TF_Status* s,
   return Const(tensor.get(), graph, s, name);
 }
 
+TF_Operation* ScalarConst(double v, TF_Graph* graph, TF_Status* s,
+                          const char* name) {
+  unique_tensor_ptr tensor(DoubleTensor(v), TF_DeleteTensor);
+  return Const(tensor.get(), graph, s, name);
+}
+
 void AddHelper(TF_Operation* l, TF_Operation* r, TF_Graph* graph, TF_Status* s,
                const char* name, TF_Operation** op, bool check) {
   TF_OperationDescription* desc = TF_NewOperation(graph, "AddN", name);

tensorflow/c/c_test_util.h

@@ -42,6 +42,8 @@ TF_Tensor* Int32Tensor(const std::vector<int32_t>& values);
 
 TF_Tensor* Int32Tensor(int32_t v);
 
+TF_Tensor* DoubleTensor(double v);
+
 TF_Operation* Placeholder(TF_Graph* graph, TF_Status* s,
                           const char* name = "feed");
 
@@ -51,6 +53,9 @@ TF_Operation* Const(TF_Tensor* t, TF_Graph* graph, TF_Status* s,
 TF_Operation* ScalarConst(int32_t v, TF_Graph* graph, TF_Status* s,
                           const char* name = "scalar");
 
+TF_Operation* ScalarConst(double v, TF_Graph* graph, TF_Status* s,
+                          const char* name = "scalar");
+
 TF_Operation* Add(TF_Operation* l, TF_Operation* r, TF_Graph* graph,
                   TF_Status* s, const char* name = "add");
 

tensorflow/c/eager/c_api.cc

@@ -226,6 +226,9 @@ TFE_TensorHandle* TFE_TensorHandleCopyToDevice(TFE_TensorHandle* h,
     tensorflow::Tensor dst(
         dstd->GetAllocator(tensorflow::AllocatorAttributes()), src->dtype(),
         src->shape());
+    if (src->shape().num_elements() == 0) {
+      return new TFE_TensorHandle(dst, dstd);
+    }
     tensorflow::Notification n;
     dstd->tensorflow_gpu_device_info()->default_context->CopyCPUTensorToDevice(
         src, dstd, &dst, [status, &n](const tensorflow::Status& s) {

tensorflow/c/while_loop_test.cc

@@ -288,20 +288,86 @@ TEST_F(CApiWhileLoopTest, NestedLoop) {
   ExpectOutputValue(1, 3);
 }
 
-TEST_F(CApiWhileLoopTest, BadCondOutput) {
+TEST_F(CApiWhileLoopTest, UnsetCondOutput) {
   Init(1);
   params_->body_outputs[0] = params_->body_inputs[0];
   ExpectError(TF_INVALID_ARGUMENT,
               "TF_WhileParams `cond_output` field isn't set");
 }
 
-TEST_F(CApiWhileLoopTest, BadBodyOutput) {
+TEST_F(CApiWhileLoopTest, WrongCondOutputType) {
+  Init(1);
+  params_->cond_output = params_->cond_inputs[0];
+  params_->body_outputs[0] = params_->body_inputs[0];
+  ExpectError(TF_INVALID_ARGUMENT,
+              "BuildWhileLoop: 'cond' argument must return a boolean output, "
+              "got int32");
+}
+
+TEST_F(CApiWhileLoopTest, InvalidCondOutputNode) {
+  Init(1);
+  // Try to reuse node from parent graph
+  params_->cond_output = inputs_[0];
+  params_->body_outputs[0] = params_->body_inputs[0];
+  // TODO(skyewm): this error message could be more informative. Add explicit
+  // checks for this case in the while loop implementation?
+  ExpectError(TF_INVALID_ARGUMENT,
+              "Requested return node 'p0' not found in graph def");
+}
+
+TEST_F(CApiWhileLoopTest, InvalidCondOutputIndex) {
+  Init(1);
+  CreateCondGraph();
+  params_->cond_output.index = 100;
+  params_->body_outputs[0] = params_->body_inputs[0];
+  ExpectError(TF_INVALID_ARGUMENT,
+              "Invalid return output 100 of node 'less_than', which has 1 "
+              "output(s)");
+}
+
+// TODO(skyewm): test bad cond output shape
+
+TEST_F(CApiWhileLoopTest, UnsetBodyOutput) {
   Init(1);
   CreateCondGraph();
   ExpectError(TF_INVALID_ARGUMENT,
               "TF_WhileParams `body_outputs[0]` field isn't set");
 }
 
+// TODO(skyewm): enable this when it works (currently doesn't error)
+// TEST_F(CApiWhileLoopTest, WrongBodyOutputType) {
+//   Init(1);
+//   CreateCondGraph();
+//   TF_Operation* double_scalar =
+//       ScalarConst(1.0, params_->body_graph, s_, "double_scalar");
+//   params_->body_outputs[0] = {double_scalar, 0};
+//   ExpectError(TF_INVALID_ARGUMENT, "bad body output type");
+// }
+
+TEST_F(CApiWhileLoopTest, InvalidBodyOutputNode) {
+  Init(1);
+  CreateCondGraph();
+  // Try to reuse node from parent graph
+  params_->body_outputs[0] = inputs_[0];
+  // TODO(skyewm): this error message could be more informative. Add explicit
+  // checks for this case in the while loop implementation?
+  ExpectError(TF_INVALID_ARGUMENT,
+              "Requested return node 'p0' not found in graph def");
+}
+
+// TODO(skyewm): enable this when it works (currently segfaults!)
+// TEST_F(CApiWhileLoopTest, InvalidBodyOutputIndex) {
+//   Init(1);
+//   CreateCondGraph();
+//   params_->body_outputs[0] = params_->body_inputs[0];
+//   params_->body_outputs[0].index = 100;
+//   ExpectError(TF_INVALID_ARGUMENT,
+//               "Invalid return output 100 of node 'less_than', which has 1 "
+//               "output(s)");
+// }
+
+// TODO(skyewm): test bad body output shape
+
 TEST_F(CApiWhileLoopTest, NullName) {
   Init(1);
   CreateCondGraph();

tensorflow/cc/BUILD

@@ -248,6 +248,21 @@ cc_library_with_android_deps(
     ],
 )
 
+tf_cc_test(
+    name = "ops_while_loop_test",
+    size = "small",
+    srcs = ["ops/while_loop_test.cc"],
+    deps = [
+        ":cc_ops",
+        ":client_session",
+        ":testutil",
+        ":while_loop",
+        "//tensorflow/core:test",
+        "//tensorflow/core:test_main",
+        "//tensorflow/core:testlib",
+    ],
+)
+
 cc_library(
     name = "grad_op_registry",
     srcs = ["framework/grad_op_registry.cc"],

tensorflow/cc/ops/while_loop.cc

@@ -102,11 +102,16 @@ Status CreateCond(const Scope& scope, const CondGraphBuilderFn& cond,
       scope.NewSubScope("cond").WithControlDependencies(inputs[0]);
   Output raw_cond_out;
   TF_RETURN_IF_ERROR(cond(cond_scope, inputs, &raw_cond_out));
+
+  TF_RETURN_IF_ERROR(scope.graph()->IsValidOutputTensor(raw_cond_out.node(),
+                                                        raw_cond_out.index()));
   if (raw_cond_out.type() != DT_BOOL) {
     return errors::InvalidArgument(
         "BuildWhileLoop: 'cond' argument must return a boolean output, got ",
         DataTypeString(raw_cond_out.type()));
   }
+  // TODO(skyewm): check that raw_cond_out is scalar
+
   *output = LoopCond(scope, raw_cond_out).output;
   return Status::OK();
 }
@@ -123,13 +128,18 @@ Status CreateBody(const Scope& scope, const BodyGraphBuilderFn& body,
   Scope body_scope =
       scope.NewSubScope("body").WithControlDependencies(inputs[0]);
   TF_RETURN_IF_ERROR(body(body_scope, inputs, outputs));
+
   const size_t num_loop_vars = inputs.size();
   if (outputs->size() != num_loop_vars) {
     return errors::InvalidArgument(
         "BuildWhileLoop: 'body' argument expected to return ", num_loop_vars,
-        "outputs, got ", outputs->size());
+        " output(s), got ", outputs->size());
+  }
+  for (const Output& output : *outputs) {
+    TF_RETURN_IF_ERROR(
+        scope.graph()->IsValidOutputTensor(output.node(), output.index()));
+    // TODO(skyewm): check output types/shapes
   }
-  // TODO(skyewm): check output types/shapes
   return Status::OK();
 }