From e0606af65fd43c3a37e28619f80dcb55bff20b77 Mon Sep 17 00:00:00 2001 From: Allen Lavoie Date: Tue, 28 Apr 2020 11:13:47 -0700 Subject: [PATCH] Small cleanups for experimental TFE attribute APIs The op name was included twice, and TFE_OpGetAttrs is unusable without a way to allocate a TFE_OpAttrs on the heap (and so has no callers). I'm removing it for now. PiperOrigin-RevId: 308859222 Change-Id: Ibb3901a1821ffc2e9ebc0efb26592e5b3d8bb88f --- tensorflow/c/eager/c_api.cc | 9 ++------- tensorflow/c/eager/c_api_experimental.h | 5 ----- tensorflow/c/eager/c_api_test.cc | 16 +++++++++++----- tensorflow/c/eager/tfe_op_attrs_internal.h | 8 +++----- 4 files changed, 16 insertions(+), 22 deletions(-) diff --git a/tensorflow/c/eager/c_api.cc b/tensorflow/c/eager/c_api.cc index c5c45378831..df950d7fc61 100644 --- a/tensorflow/c/eager/c_api.cc +++ b/tensorflow/c/eager/c_api.cc @@ -1485,11 +1485,6 @@ void TFE_ContextEndStep(TFE_Context* ctx) { context->EndStep(); } -void TFE_OpGetAttrs(TFE_Op* op, TFE_OpAttrs* attrs) { - tensorflow::EagerOperation* operation = OperationFromInterface(op->operation); - *attrs = TFE_OpAttrs(&operation->Attrs(), operation->Name().c_str()); -} - void TFE_OpAddAttrs(TFE_Op* op, const TFE_OpAttrs* attrs) { tensorflow::AttrValueMap m; attrs->attributes->FillAttrValueMap(&m); @@ -1504,7 +1499,7 @@ void TFE_OpAttrsSerialize(const TFE_OpAttrs* attrs, TF_Buffer* buf, TF_Status* status) { tensorflow::NameAttrList name_and_attrs; attrs->attributes->FillAttrValueMap(name_and_attrs.mutable_attr()); - name_and_attrs.set_name(attrs->name); + name_and_attrs.set_name(attrs->attributes->op_name()); status->status = MessageToBuffer(name_and_attrs, buf); } @@ -1624,7 +1619,7 @@ class CustomDeviceAPI : public tensorflow::CustomDevice { } std::vector outputs(*num_retvals); TF_Status status; - TFE_OpAttrs attributes(&op->Attrs(), op->Name().c_str()); + TFE_OpAttrs attributes(&op->Attrs()); device_.execute(context_, inputs.size(), inputs.data(), op->Name().c_str(), &attributes, num_retvals, outputs.data(), &status, info_); if (status.status.ok()) { diff --git a/tensorflow/c/eager/c_api_experimental.h b/tensorflow/c/eager/c_api_experimental.h index dc1f9eaade3..d1e99d86180 100644 --- a/tensorflow/c/eager/c_api_experimental.h +++ b/tensorflow/c/eager/c_api_experimental.h @@ -431,11 +431,6 @@ TF_CAPI_EXPORT extern void TFE_HostAddressSpace(TFE_Context* ctx, // A reference to an op's name -> attribute mapping typedef struct TFE_OpAttrs TFE_OpAttrs; -// Fetch a struct with a reference to information about attributes of `op`. -// -// The `attrs` struct does not own any memory, and `op` must outlive it. -TF_CAPI_EXPORT extern void TFE_OpGetAttrs(TFE_Op* op, TFE_OpAttrs* attrs); - // Add attributes in `attrs` to `op`. // // Does not overwrite or update existing attributes, but adds new ones. diff --git a/tensorflow/c/eager/c_api_test.cc b/tensorflow/c/eager/c_api_test.cc index 0b5a0ae8c08..8abbe1382ad 100644 --- a/tensorflow/c/eager/c_api_test.cc +++ b/tensorflow/c/eager/c_api_test.cc @@ -1577,7 +1577,7 @@ TEST(CAPI, TestTFE_OpGetInputAndOutputLengthsFailForUnknownArguments) { TFE_DeleteContext(ctx); } -TEST(CAPI, TestTFE_OpGetAttrs) { +TEST(CAPI, TestTFE_OpAddAttrs) { TF_Status* status = TF_NewStatus(); TFE_ContextOptions* opts = TFE_NewContextOptions(); TFE_Context* ctx = TFE_NewContext(opts, status); @@ -1587,8 +1587,11 @@ TEST(CAPI, TestTFE_OpGetAttrs) { TFE_Op* var_op = TFE_NewOp(ctx, "VarHandleOp", status); TFE_OpSetAttrType(var_op, "dtype", TF_INT64); TFE_OpSetAttrShape(var_op, "shape", {}, 0, status); - TFE_OpAttrs attributes; - TFE_OpGetAttrs(var_op, &attributes); + // There is currently no API to fetch attributes from an operation, fetching + // happens only as an implementation detail of custom devices. + tensorflow::EagerOperation* operation = + OperationFromInterface(var_op->operation); + TFE_OpAttrs attributes{&operation->Attrs()}; TFE_Op* copy_op = TFE_NewOp(ctx, "VarHandleOp", status); TFE_OpSetAttrType(copy_op, "dtype", TF_FLOAT); @@ -1624,8 +1627,11 @@ TEST(CAPI, TestTFE_OpAttrsSerialize) { CHECK_EQ(TF_OK, TF_GetCode(status)) << TF_Message(status); TFE_OpSetAttrType(var_op, "dtype", TF_INT64); TFE_OpSetAttrShape(var_op, "shape", {}, 0, status); - TFE_OpAttrs attributes; - TFE_OpGetAttrs(var_op, &attributes); + // There is currently no API to fetch attributes from an operation, fetching + // happens only as an implementation detail of custom devices. + tensorflow::EagerOperation* operation = + OperationFromInterface(var_op->operation); + TFE_OpAttrs attributes{&operation->Attrs()}; TF_Buffer* serialized_attr_values = TF_NewBuffer(); TFE_OpAttrsSerialize(&attributes, serialized_attr_values, status); diff --git a/tensorflow/c/eager/tfe_op_attrs_internal.h b/tensorflow/c/eager/tfe_op_attrs_internal.h index 5351da5380e..1d745c0ce4f 100644 --- a/tensorflow/c/eager/tfe_op_attrs_internal.h +++ b/tensorflow/c/eager/tfe_op_attrs_internal.h @@ -32,13 +32,11 @@ limitations under the License. // An equivalent of a tensorflow::NameAttrList protocol buffer, but used in ways // that sometimes do not require serialization. struct TFE_OpAttrs { - explicit TFE_OpAttrs() : name(nullptr), attributes(nullptr) {} + explicit TFE_OpAttrs() : attributes(nullptr) {} - explicit TFE_OpAttrs(const tensorflow::AttrBuilder* value, - const char* op_name) - : name(op_name), attributes(value) {} + explicit TFE_OpAttrs(const tensorflow::AttrBuilder* value) + : attributes(value) {} - const char* name; const tensorflow::AttrBuilder* attributes; };