From 2623d9a80992128db89a536248a74679bfbe7517 Mon Sep 17 00:00:00 2001 From: Daniel Nguyen Date: Tue, 21 Jul 2020 19:54:07 +0000 Subject: [PATCH 1/4] added TF_GetName and tests for the funciton --- tensorflow/c/kernels.cc | 10 +++++++++- tensorflow/c/kernels.h | 17 +++++++++++++++++ tensorflow/c/kernels_test.cc | 16 ++++++++++++++-- 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/tensorflow/c/kernels.cc b/tensorflow/c/kernels.cc index 8fa50711a8d..42aa2bce54a 100644 --- a/tensorflow/c/kernels.cc +++ b/tensorflow/c/kernels.cc @@ -239,6 +239,14 @@ void TF_OpKernelContext_Failure(TF_OpKernelContext* ctx, TF_Status* status) { DEFINE_TF_GETATTR(Type, TF_DataType, tensorflow::DataType) DEFINE_TF_GETATTR(Int32, tensorflow::int32, int32_t) +string_view TF_OpKernelConstruction_GetName(TF_OpKernelConstruction* ctx) { + auto* cc_ctx = reinterpret_cast(ctx); + string_view string_view_of_name = {.data = cc_ctx->def().name().data(), + .len = cc_ctx->def().name().length()}; + return string_view_of_name; +} + + TF_DataType TF_ExpectedOutputDataType(TF_OpKernelContext* ctx, int i) { auto* cc_ctx = reinterpret_cast<::tensorflow::OpKernelContext*>(ctx); return static_cast(cc_ctx->expected_output_dtype(i)); @@ -271,4 +279,4 @@ TF_Tensor* TF_AllocateOutput(TF_OpKernelContext* context, int index, return nullptr; } return tf_tensor; -} +} \ No newline at end of file diff --git a/tensorflow/c/kernels.h b/tensorflow/c/kernels.h index 1428f7ab928..e146e896485 100644 --- a/tensorflow/c/kernels.h +++ b/tensorflow/c/kernels.h @@ -111,6 +111,10 @@ TF_CAPI_EXPORT extern void TF_KernelBuilder_HostMemory( TF_CAPI_EXPORT extern void TF_KernelBuilder_Priority( TF_KernelBuilder* kernel_builder, int32_t priority_number); +typedef struct string_view string_view; + +TF_CAPI_EXPORT extern string_view TF_GetName(TF_KernelBuilder* kernel_builder); + // Register the given kernel builder with the TensorFlow runtime. If // registration fails, the given status will be populated. // @@ -184,6 +188,19 @@ TF_CAPI_EXPORT extern void TF_OpKernelConstruction_GetAttrInt32( TF_OpKernelConstruction* ctx, const char* attr_name, int32_t* val, TF_Status* status); +// Used to pass strings across the C API. The caller does not take ownership +// of the underlying data pointer and is not responsible for freeing it. +struct string_view { + const char* data; + size_t len; +}; + +typedef struct string_view string_view; + +// Returns the name of the user-defined NodeDef for this OpKernel. +TF_CAPI_EXPORT extern string_view TF_OpKernelConstruction_GetName( + TF_OpKernelConstruction* ctx); + // Allocates Tensor for output at given index. Caller takes ownership of // returned TF_Tensor and should deallocate it using TF_DeleteTensor(tensor). // diff --git a/tensorflow/c/kernels_test.cc b/tensorflow/c/kernels_test.cc index 423302741de..ddbb86d2f31 100644 --- a/tensorflow/c/kernels_test.cc +++ b/tensorflow/c/kernels_test.cc @@ -53,6 +53,7 @@ limitations under the License. #include "tensorflow/core/platform/test.h" #include "tensorflow/core/platform/types.h" +#include struct MyCustomKernel { bool created; bool compute_called; @@ -73,6 +74,12 @@ static void* MyCreateFunc(TF_OpKernelConstruction* ctx) { EXPECT_EQ(TF_FLOAT, type); TF_DeleteStatus(status); + // Exercise kernel NodeDef name read + string_view name_string_view = TF_OpKernelConstruction_GetName(ctx); + const char* kernel_name = "SomeKernelName"; + const char* candidate_kernel_name = std::string(name_string_view.data, + name_string_view.len).c_str(); + EXPECT_EQ(0, strcmp(kernel_name, candidate_kernel_name)); return s; } @@ -96,9 +103,11 @@ namespace tensorflow { static std::unique_ptr GetFakeKernel(const char* device_name, const char* op_name, + const char* kernel_name, Status* status) { NodeDef def; def.set_op(op_name); + def.set_name(kernel_name); def.set_device(device_name); def.add_input("input1"); def.add_input("input2"); @@ -144,7 +153,7 @@ TEST(TestKernel, TestRegisterKernelBuilder) { { Status status; std::unique_ptr kernel = - GetFakeKernel(device_name, op_name, &status); + GetFakeKernel(device_name, op_name, kernel_name, &status); TF_EXPECT_OK(status); ASSERT_NE(nullptr, kernel.get()); kernel->Compute(nullptr); @@ -153,6 +162,9 @@ TEST(TestKernel, TestRegisterKernelBuilder) { ASSERT_TRUE(delete_called); } +TEST(TestKernel, TestGetKernelName) { +} + class DummyDevice : public DeviceBase { public: explicit DummyDevice(Env* env) : DeviceBase(env) {} @@ -233,7 +245,7 @@ TEST(TestKernel, TestInputAndOutputCount) { Status status; std::unique_ptr kernel = - GetFakeKernel(device_name, op_name, &status); + GetFakeKernel(device_name, op_name, kernel_name, &status); TF_EXPECT_OK(status); ASSERT_NE(nullptr, kernel.get()); From 540e9cf481aed11012e6803df7fb415bc2dcca2a Mon Sep 17 00:00:00 2001 From: Daniel Nguyen Date: Wed, 22 Jul 2020 00:28:36 +0000 Subject: [PATCH 2/4] moved TF_StringView to c_api.h and manually declared TF_StringView members in GetName --- tensorflow/c/c_api.h | 8 ++++++++ tensorflow/c/kernels.cc | 8 ++++---- tensorflow/c/kernels.h | 13 +++--------- tensorflow/c/kernels_test.cc | 38 ++++++++++++++++++------------------ 4 files changed, 34 insertions(+), 33 deletions(-) diff --git a/tensorflow/c/c_api.h b/tensorflow/c/c_api.h index 808bcf3bd80..c6ca319c40d 100644 --- a/tensorflow/c/c_api.h +++ b/tensorflow/c/c_api.h @@ -125,6 +125,14 @@ TF_CAPI_EXPORT extern void TF_DeleteBuffer(TF_Buffer*); TF_CAPI_EXPORT extern TF_Buffer TF_GetBuffer(TF_Buffer* buffer); +// -------------------------------------------------------------------------- +// Used to pass strings across the C API. The caller does not take ownership +// of the underlying data pointer and is not responsible for freeing it. +struct TF_StringView { + const char* data; + size_t len; +}; + // -------------------------------------------------------------------------- // TF_SessionOptions holds options that can be passed during session creation. typedef struct TF_SessionOptions TF_SessionOptions; diff --git a/tensorflow/c/kernels.cc b/tensorflow/c/kernels.cc index 42aa2bce54a..6b8348aaa3d 100644 --- a/tensorflow/c/kernels.cc +++ b/tensorflow/c/kernels.cc @@ -239,14 +239,14 @@ void TF_OpKernelContext_Failure(TF_OpKernelContext* ctx, TF_Status* status) { DEFINE_TF_GETATTR(Type, TF_DataType, tensorflow::DataType) DEFINE_TF_GETATTR(Int32, tensorflow::int32, int32_t) -string_view TF_OpKernelConstruction_GetName(TF_OpKernelConstruction* ctx) { +TF_StringView TF_OpKernelConstruction_GetName(TF_OpKernelConstruction* ctx) { auto* cc_ctx = reinterpret_cast(ctx); - string_view string_view_of_name = {.data = cc_ctx->def().name().data(), - .len = cc_ctx->def().name().length()}; + TF_StringView string_view_of_name; + string_view_of_name.data = cc_ctx->def().name().data(); + string_view_of_name.len = cc_ctx->def().name().length(); return string_view_of_name; } - TF_DataType TF_ExpectedOutputDataType(TF_OpKernelContext* ctx, int i) { auto* cc_ctx = reinterpret_cast<::tensorflow::OpKernelContext*>(ctx); return static_cast(cc_ctx->expected_output_dtype(i)); diff --git a/tensorflow/c/kernels.h b/tensorflow/c/kernels.h index 7e2a3aa5067..cec0ca7b6a2 100644 --- a/tensorflow/c/kernels.h +++ b/tensorflow/c/kernels.h @@ -20,6 +20,7 @@ limitations under the License. #include "tensorflow/c/tf_datatype.h" #include "tensorflow/c/tf_status.h" +#include "tensorflow/c/c_api.h" // Macro to control visibility of exported symbols in the shared library (.so, // .dylib, .dll). @@ -45,6 +46,7 @@ extern "C" { #endif typedef struct TF_Tensor TF_Tensor; +typedef struct TF_StringView TF_StringView; // -------------------------------------------------------------------------- // C API for TensorFlow Kernels. @@ -184,17 +186,8 @@ TF_CAPI_EXPORT extern void TF_OpKernelConstruction_GetAttrInt32( TF_OpKernelConstruction* ctx, const char* attr_name, int32_t* val, TF_Status* status); -// Used to pass strings across the C API. The caller does not take ownership -// of the underlying data pointer and is not responsible for freeing it. -struct string_view { - const char* data; - size_t len; -}; - -typedef struct string_view string_view; - // Returns the name of the user-defined NodeDef for this OpKernel. -TF_CAPI_EXPORT extern string_view TF_OpKernelConstruction_GetName( +TF_CAPI_EXPORT extern TF_StringView TF_OpKernelConstruction_GetName( TF_OpKernelConstruction* ctx); // Allocates Tensor for output at given index. Caller takes ownership of diff --git a/tensorflow/c/kernels_test.cc b/tensorflow/c/kernels_test.cc index b01e3ea3fac..4a3b83e240c 100644 --- a/tensorflow/c/kernels_test.cc +++ b/tensorflow/c/kernels_test.cc @@ -74,11 +74,11 @@ static void* MyCreateFunc(TF_OpKernelConstruction* ctx) { TF_DeleteStatus(status); // Exercise kernel NodeDef name read - string_view name_string_view = TF_OpKernelConstruction_GetName(ctx); - const char* kernel_name = "SomeKernelName"; - const char* candidate_kernel_name = std::string(name_string_view.data, - name_string_view.len).c_str(); - EXPECT_EQ(0, strcmp(kernel_name, candidate_kernel_name)); + TF_StringView name_string_view = TF_OpKernelConstruction_GetName(ctx); + std::string node_name = "SomeNodeName"; + std::string candidate_node_name = std::string(name_string_view.data, + name_string_view.len); + EXPECT_EQ(node_name, candidate_node_name); return s; } @@ -102,11 +102,11 @@ namespace tensorflow { static std::unique_ptr GetFakeKernel(const char* device_name, const char* op_name, - const char* kernel_name, + const char* node_name, Status* status) { NodeDef def; def.set_op(op_name); - def.set_name(kernel_name); + def.set_name(node_name); def.set_device(device_name); def.add_input("input1"); def.add_input("input2"); @@ -122,7 +122,7 @@ static std::unique_ptr GetFakeKernel(const char* device_name, // Tests registration of a single C kernel and checks that calls through the // C/C++ boundary are being made. TEST(TestKernel, TestRegisterKernelBuilder) { - const char* kernel_name = "SomeKernelName"; + const char* node_name = "SomeNodeName"; const char* op_name = "FooOp"; const char* device_name = "FakeDeviceName1"; @@ -137,7 +137,7 @@ TEST(TestKernel, TestRegisterKernelBuilder) { { TF_Status* status = TF_NewStatus(); - TF_RegisterKernelBuilder(kernel_name, builder, status); + TF_RegisterKernelBuilder(node_name, builder, status); EXPECT_EQ(TF_OK, TF_GetCode(status)); TF_Buffer* buf = TF_GetRegisteredKernelsForOp(op_name, status); EXPECT_EQ(TF_OK, TF_GetCode(status)); @@ -152,7 +152,7 @@ TEST(TestKernel, TestRegisterKernelBuilder) { { Status status; std::unique_ptr kernel = - GetFakeKernel(device_name, op_name, kernel_name, &status); + GetFakeKernel(device_name, op_name, node_name, &status); TF_EXPECT_OK(status); ASSERT_NE(nullptr, kernel.get()); kernel->Compute(nullptr); @@ -170,7 +170,7 @@ class DummyDevice : public DeviceBase { }; TEST(TestKernel, TestInputAndOutputCount) { - const char* kernel_name = "InputOutputCounterKernel"; + const char* node_name = "InputOutputCounterKernel"; const char* op_name = "BarOp"; const char* device_name = "FakeDeviceName2"; @@ -220,7 +220,7 @@ TEST(TestKernel, TestInputAndOutputCount) { { TF_Status* status = TF_NewStatus(); - TF_RegisterKernelBuilder(kernel_name, builder, status); + TF_RegisterKernelBuilder(node_name, builder, status); EXPECT_EQ(TF_OK, TF_GetCode(status)); TF_DeleteStatus(status); } @@ -241,7 +241,7 @@ TEST(TestKernel, TestInputAndOutputCount) { Status status; std::unique_ptr kernel = - GetFakeKernel(device_name, op_name, kernel_name, &status); + GetFakeKernel(device_name, op_name, node_name, &status); TF_EXPECT_OK(status); ASSERT_NE(nullptr, kernel.get()); @@ -260,7 +260,7 @@ TEST(TestKernel, DeleteKernelBuilderIsOkOnNull) { } TEST(TestKernel, TestTypeConstraint) { - const char* kernel_name = "SomeKernelName"; + const char* node_name = "SomeNodeName"; const char* op_name = "TypeOp"; const char* device_name = "FakeDeviceName1"; @@ -275,7 +275,7 @@ TEST(TestKernel, TestTypeConstraint) { TF_Status* status = TF_NewStatus(); TF_KernelBuilder_TypeConstraint(builder, "T", TF_DataType::TF_INT32, status); EXPECT_EQ(TF_OK, TF_GetCode(status)); - TF_RegisterKernelBuilder(kernel_name, builder, status); + TF_RegisterKernelBuilder(node_name, builder, status); EXPECT_EQ(TF_OK, TF_GetCode(status)); TF_Buffer* buf = TF_GetRegisteredKernelsForOp(op_name, status); @@ -304,7 +304,7 @@ TEST(TestKernel, TestTypeConstraint) { } TEST(TestKernel, TestHostMemory) { - const char* kernel_name = "SomeKernelName"; + const char* node_name = "SomeNodeName"; const char* op_name = "HostMemoryOp"; const char* device_name = "FakeDeviceName1"; @@ -319,7 +319,7 @@ TEST(TestKernel, TestHostMemory) { TF_KernelBuilder_HostMemory(builder, "input2"); TF_KernelBuilder_HostMemory(builder, "output1"); TF_Status* status = TF_NewStatus(); - TF_RegisterKernelBuilder(kernel_name, builder, status); + TF_RegisterKernelBuilder(node_name, builder, status); EXPECT_EQ(TF_OK, TF_GetCode(status)); TF_Buffer* buf = TF_GetRegisteredKernelsForOp(op_name, status); @@ -343,12 +343,12 @@ TEST(TestKernel, TestHostMemory) { class DeviceKernelOpTest : public OpsTestBase { protected: - void SetupOp(const char* op_name, const char* kernel_name, + void SetupOp(const char* op_name, const char* node_name, void (*compute_func)(void*, TF_OpKernelContext*)) { TF_KernelBuilder* builder = TF_NewKernelBuilder( op_name, device_name_, nullptr, compute_func, nullptr); TF_Status* status = TF_NewStatus(); - TF_RegisterKernelBuilder(kernel_name, builder, status); + TF_RegisterKernelBuilder(node_name, builder, status); EXPECT_EQ(TF_OK, TF_GetCode(status)); TF_DeleteStatus(status); From 503defb219c271c23897371fb837daae25c507bb Mon Sep 17 00:00:00 2001 From: Daniel Nguyen Date: Wed, 22 Jul 2020 17:53:56 +0000 Subject: [PATCH 3/4] moved typedef to c_api.h --- tensorflow/c/c_api.h | 4 ++-- tensorflow/c/kernels.h | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tensorflow/c/c_api.h b/tensorflow/c/c_api.h index c6ca319c40d..e49bf3601ec 100644 --- a/tensorflow/c/c_api.h +++ b/tensorflow/c/c_api.h @@ -128,10 +128,10 @@ TF_CAPI_EXPORT extern TF_Buffer TF_GetBuffer(TF_Buffer* buffer); // -------------------------------------------------------------------------- // Used to pass strings across the C API. The caller does not take ownership // of the underlying data pointer and is not responsible for freeing it. -struct TF_StringView { +typedef struct TF_StringView { const char* data; size_t len; -}; +} TF_StringView; // -------------------------------------------------------------------------- // TF_SessionOptions holds options that can be passed during session creation. diff --git a/tensorflow/c/kernels.h b/tensorflow/c/kernels.h index cec0ca7b6a2..764f9066faa 100644 --- a/tensorflow/c/kernels.h +++ b/tensorflow/c/kernels.h @@ -46,7 +46,6 @@ extern "C" { #endif typedef struct TF_Tensor TF_Tensor; -typedef struct TF_StringView TF_StringView; // -------------------------------------------------------------------------- // C API for TensorFlow Kernels. From c3704aaacb7a699a45d5ef25584268dfc884cd26 Mon Sep 17 00:00:00 2001 From: Daniel Nguyen Date: Thu, 23 Jul 2020 18:07:44 +0000 Subject: [PATCH 4/4] made comments for string view and get_name more clear --- tensorflow/c/c_api.h | 2 +- tensorflow/c/kernels.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tensorflow/c/c_api.h b/tensorflow/c/c_api.h index e49bf3601ec..348c84fcd65 100644 --- a/tensorflow/c/c_api.h +++ b/tensorflow/c/c_api.h @@ -126,7 +126,7 @@ TF_CAPI_EXPORT extern void TF_DeleteBuffer(TF_Buffer*); TF_CAPI_EXPORT extern TF_Buffer TF_GetBuffer(TF_Buffer* buffer); // -------------------------------------------------------------------------- -// Used to pass strings across the C API. The caller does not take ownership +// Used to return strings across the C API. The caller does not take ownership // of the underlying data pointer and is not responsible for freeing it. typedef struct TF_StringView { const char* data; diff --git a/tensorflow/c/kernels.h b/tensorflow/c/kernels.h index 764f9066faa..d7b19ec91e4 100644 --- a/tensorflow/c/kernels.h +++ b/tensorflow/c/kernels.h @@ -185,7 +185,7 @@ TF_CAPI_EXPORT extern void TF_OpKernelConstruction_GetAttrInt32( TF_OpKernelConstruction* ctx, const char* attr_name, int32_t* val, TF_Status* status); -// Returns the name of the user-defined NodeDef for this OpKernel. +// Returns the unique operation name for this OpKernel. TF_CAPI_EXPORT extern TF_StringView TF_OpKernelConstruction_GetName( TF_OpKernelConstruction* ctx);