From 57b3a5098465228b8533e3a04dd363be282a2baf Mon Sep 17 00:00:00 2001 From: Gary Linscott Date: Fri, 6 Dec 2024 13:56:52 -0800 Subject: [PATCH 1/3] Revert "Bump the absl version to match tensorflow. 2.15." This reverts commit 56a4e5acd9a24308816517d84f3ed5bc737ccfc1. --- reverb/cc/platform/default/repo.bzl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/reverb/cc/platform/default/repo.bzl b/reverb/cc/platform/default/repo.bzl index 3f8e678..d1b7381 100644 --- a/reverb/cc/platform/default/repo.bzl +++ b/reverb/cc/platform/default/repo.bzl @@ -360,11 +360,11 @@ def googletest_deps(): def absl_deps(): http_archive( name = "com_google_absl", - sha256 = "0320586856674d16b0b7a4d4afb22151bdc798490bb7f295eddd8f6a62b46fea", # SHARED_ABSL_SHA - strip_prefix = "abseil-cpp-fb3621f4f897824c0dbe0615fa94543df6192f30", + sha256 = "8eeec9382fc0338ef5c60053f3a4b0e0708361375fe51c9e65d0ce46ccfe55a7", # SHARED_ABSL_SHA + strip_prefix = "abseil-cpp-b971ac5250ea8de900eae9f95e06548d14cd95fe", urls = [ - "https://storage.googleapis.com/mirror.tensorflow.org/github.com/abseil/abseil-cpp/archive/fb3621f4f897824c0dbe0615fa94543df6192f30.tar.gz", - "https://github.com/abseil/abseil-cpp/archive/fb3621f4f897824c0dbe0615fa94543df6192f30.tar.gz", + "https://storage.googleapis.com/mirror.tensorflow.org/github.com/abseil/abseil-cpp/archive/b971ac5250ea8de900eae9f95e06548d14cd95fe.tar.gz", + "https://github.com/abseil/abseil-cpp/archive/b971ac5250ea8de900eae9f95e06548d14cd95fe.tar.gz", ], ) From 59f52bca5e11f01ea9f045f2d341307703c386e7 Mon Sep 17 00:00:00 2001 From: Gary Linscott Date: Fri, 6 Dec 2024 14:43:02 -0800 Subject: [PATCH 2/3] Compiling against TF 2.15.1 --- reverb/cc/BUILD | 19 ++++++++-------- reverb/cc/platform/default/build_rules.bzl | 1 + reverb/cc/sampler_test.cc | 5 +---- reverb/cc/support/trajectory_util_test.cc | 4 ++-- reverb/cc/tensor_compression_test.cc | 26 +++++++++++++--------- reverb/cc/testing/BUILD | 2 +- reverb/cc/writer.cc | 5 +++-- 7 files changed, 34 insertions(+), 28 deletions(-) diff --git a/reverb/cc/BUILD b/reverb/cc/BUILD index 9bba2db..503fd0e 100644 --- a/reverb/cc/BUILD +++ b/reverb/cc/BUILD @@ -67,7 +67,8 @@ reverb_cc_test( deps = [ ":tensor_compression", "//reverb/cc/testing:tensor_testutil", - "//third_party/absl/status", + "//reverb/cc/platform:status_matchers", + # "//third_party/absl/status", ] + reverb_tf_deps(), ) @@ -88,8 +89,8 @@ reverb_cc_test( "//reverb/cc/testing:proto_test_util", "//reverb/cc/testing:tensor_testutil", "//reverb/cc/testing:time_testutil", - "//third_party/absl/log:check", - "//third_party/grpc:grpc++", + # "//third_party/absl/log:check", + # "//third_party/grpc:grpc++", ] + reverb_tf_deps() + reverb_grpc_deps() + reverb_absl_deps(), ) @@ -191,10 +192,10 @@ reverb_cc_library( deps = [ "//reverb/cc/platform:logging", "//reverb/cc/platform:snappy", - "//third_party/absl/status", - "//third_party/absl/status:statusor", - "//third_party/absl/strings", - "//third_party/tensorflow/tsl/platform:status", + # "//third_party/absl/status", + # "//third_party/absl/status:statusor", + # "//third_party/absl/strings", + # "//third_party/tensorflow/tsl/platform:status", ] + reverb_tf_deps(), ) @@ -241,8 +242,8 @@ reverb_cc_library( "//reverb/cc/support:signature", "//reverb/cc/support:tf_util", "//reverb/cc/support:trajectory_util", - "//third_party/absl/log:check", - "//third_party/grpc:grpc++", + # "//third_party/absl/log:check", + # "//third_party/grpc:grpc++", ] + reverb_tf_deps() + reverb_grpc_deps() + reverb_absl_deps(), ) diff --git a/reverb/cc/platform/default/build_rules.bzl b/reverb/cc/platform/default/build_rules.bzl index 65ec210..5fab78f 100644 --- a/reverb/cc/platform/default/build_rules.bzl +++ b/reverb/cc/platform/default/build_rules.bzl @@ -567,6 +567,7 @@ def reverb_absl_deps(): "@com_google_absl//absl/base", "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/functional:bind_front", + "@com_google_absl//absl/log:check", "@com_google_absl//absl/memory", "@com_google_absl//absl/numeric:int128", "@com_google_absl//absl/random", diff --git a/reverb/cc/sampler_test.cc b/reverb/cc/sampler_test.cc index 5103abe..d66c9ec 100644 --- a/reverb/cc/sampler_test.cc +++ b/reverb/cc/sampler_test.cc @@ -35,9 +35,6 @@ #include "absl/synchronization/mutex.h" #include "absl/time/clock.h" #include "absl/time/time.h" -#include "third_party/grpc/include/grpcpp/client_context.h" -#include "third_party/grpc/include/grpcpp/impl/call_op_set.h" -#include "third_party/grpc/include/grpcpp/support/sync_stream.h" #include "reverb/cc/chunk_store.h" #include "reverb/cc/platform/logging.h" #include "reverb/cc/platform/status_matchers.h" @@ -703,7 +700,7 @@ TEST(GrpcSamplerTest, GetNextTimestepReturnsErrorIfNotDecomposible) { auto* entry = response.mutable_entries(0); // Add a column of length 10 to the existing one of length 5. - ASSERT_OK(CompressTensorAsProto( + REVERB_ASSERT_OK(CompressTensorAsProto( MakeTensor(10), entry->add_data()->mutable_data()->add_tensors())); auto* slice = entry->mutable_info() ->mutable_item() diff --git a/reverb/cc/support/trajectory_util_test.cc b/reverb/cc/support/trajectory_util_test.cc index d9d52e9..236a948 100644 --- a/reverb/cc/support/trajectory_util_test.cc +++ b/reverb/cc/support/trajectory_util_test.cc @@ -246,9 +246,9 @@ TEST(UnpackChunkColumn, SelectsCorrectColumn) { tensorflow::Tensor second_col_tensor(static_cast(9000)); ChunkData data; - ASSERT_OK(CompressTensorAsProto(first_col_tensor, + REVERB_ASSERT_OK(CompressTensorAsProto(first_col_tensor, data.mutable_data()->add_tensors())); - ASSERT_OK(CompressTensorAsProto(second_col_tensor, + REVERB_ASSERT_OK(CompressTensorAsProto(second_col_tensor, data.mutable_data()->add_tensors())); data.set_data_tensors_len(2); diff --git a/reverb/cc/tensor_compression_test.cc b/reverb/cc/tensor_compression_test.cc index 6bba0c4..189e4c0 100644 --- a/reverb/cc/tensor_compression_test.cc +++ b/reverb/cc/tensor_compression_test.cc @@ -19,6 +19,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" #include "absl/status/status.h" +#include "reverb/cc/platform/status_matchers.h" #include "reverb/cc/testing/tensor_testutil.h" #include "tensorflow/core/framework/register_types.h" #include "tensorflow/core/framework/tensor.h" @@ -28,13 +29,18 @@ #include "tensorflow/core/framework/types.pb.h" #include "tensorflow/core/framework/variant.h" #include "tensorflow/core/platform/tstring.h" +#include "tensorflow/core/framework/variant_encode_decode.h" namespace deepmind { namespace reverb { namespace { using ::testing::HasSubstr; -using ::testing::status::StatusIs; +// using ::testing::status::StatusIs; + +MATCHER_P2(StatusIs, code, message, "") { + return arg.code() == code && absl::StrContains(arg.message(), message); +} template void EncodeMatchesDecodeT() { @@ -75,9 +81,9 @@ TEST(TensorCompressionTest, StringTensor) { tensor.flat()(1) = "world"; tensorflow::TensorProto proto; - ASSERT_OK(CompressTensorAsProto(tensor, &proto)); + REVERB_ASSERT_OK(CompressTensorAsProto(tensor, &proto)); - ASSERT_OK_AND_ASSIGN(tensorflow::Tensor result, + TF_ASSERT_OK_AND_ASSIGN(tensorflow::Tensor result, DecompressTensorFromProto(proto)); test::ExpectTensorEqual(tensor, result); } @@ -88,9 +94,9 @@ TEST(TensorCompressionTest, NonStringTensor) { tensor.flat().setRandom(); tensorflow::TensorProto proto; - ASSERT_OK(CompressTensorAsProto(tensor, &proto)); + REVERB_ASSERT_OK(CompressTensorAsProto(tensor, &proto)); - ASSERT_OK_AND_ASSIGN(tensorflow::Tensor result, + TF_ASSERT_OK_AND_ASSIGN(tensorflow::Tensor result, DecompressTensorFromProto(proto)); test::ExpectTensorEqual(tensor, result); } @@ -101,9 +107,9 @@ TEST(TensorCompressionTest, NonStringTensorWithDeltaEncoding) { tensor.flat().setRandom(); tensorflow::TensorProto proto; - ASSERT_OK(CompressTensorAsProto(DeltaEncode(tensor, true), &proto)); + REVERB_ASSERT_OK(CompressTensorAsProto(DeltaEncode(tensor, true), &proto)); - ASSERT_OK_AND_ASSIGN(tensorflow::Tensor result, + TF_ASSERT_OK_AND_ASSIGN(tensorflow::Tensor result, DecompressTensorFromProto(proto)); test::ExpectTensorEqual(tensor, DeltaEncode(result, false)); } @@ -120,16 +126,16 @@ TEST(TensorCompressionTest, CompressingVariantNotSupported) { tensorflow::TensorProto proto; EXPECT_THAT(CompressTensorAsProto(DeltaEncode(tensor, true), &proto), StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("variant is not supported"))); + "variant is not supported")); } TEST(TensorCompressionTest, DecompressingVariantNotSupported) { tensorflow::TensorProto proto; proto.set_dtype(tensorflow::DT_VARIANT); - EXPECT_THAT(DecompressTensorFromProto(proto), + EXPECT_THAT(DecompressTensorFromProto(proto).status(), StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("variant is not supported"))); + "variant is not supported")); } } // namespace diff --git a/reverb/cc/testing/BUILD b/reverb/cc/testing/BUILD index b481aea..21ba89e 100644 --- a/reverb/cc/testing/BUILD +++ b/reverb/cc/testing/BUILD @@ -18,7 +18,7 @@ reverb_cc_library( "//reverb/cc:schema_cc_proto", "//reverb/cc:tensor_compression", "//reverb/cc/platform:logging", - "//third_party/absl/log:check", + # "//third_party/absl/log:check", ] + reverb_tf_deps(), ) diff --git a/reverb/cc/writer.cc b/reverb/cc/writer.cc index d458542..1ba5f04 100644 --- a/reverb/cc/writer.cc +++ b/reverb/cc/writer.cc @@ -33,8 +33,9 @@ #include "absl/strings/string_view.h" #include "absl/synchronization/mutex.h" #include "absl/types/optional.h" -#include "third_party/grpc/include/grpcpp/client_context.h" -#include "third_party/grpc/include/grpcpp/impl/call_op_set.h" +#include "grpcpp/impl/codegen/client_context.h" +// #include "third_party/grpc/include/grpcpp/client_context.h" +// #include "third_party/grpc/include/grpcpp/impl/call_op_set.h" #include "reverb/cc/platform/hash_set.h" #include "reverb/cc/platform/logging.h" #include "reverb/cc/platform/status_macros.h" From 271016e71c2e8d1a780864bcb8d2d90f41f11232 Mon Sep 17 00:00:00 2001 From: Gary Linscott Date: Mon, 9 Dec 2024 09:43:13 -0800 Subject: [PATCH 3/3] cleanup --- reverb/cc/BUILD | 9 --------- reverb/cc/tensor_compression_test.cc | 1 - reverb/cc/testing/BUILD | 1 - reverb/cc/writer.cc | 2 -- 4 files changed, 13 deletions(-) diff --git a/reverb/cc/BUILD b/reverb/cc/BUILD index 503fd0e..caa3e37 100644 --- a/reverb/cc/BUILD +++ b/reverb/cc/BUILD @@ -68,7 +68,6 @@ reverb_cc_test( ":tensor_compression", "//reverb/cc/testing:tensor_testutil", "//reverb/cc/platform:status_matchers", - # "//third_party/absl/status", ] + reverb_tf_deps(), ) @@ -89,8 +88,6 @@ reverb_cc_test( "//reverb/cc/testing:proto_test_util", "//reverb/cc/testing:tensor_testutil", "//reverb/cc/testing:time_testutil", - # "//third_party/absl/log:check", - # "//third_party/grpc:grpc++", ] + reverb_tf_deps() + reverb_grpc_deps() + reverb_absl_deps(), ) @@ -192,10 +189,6 @@ reverb_cc_library( deps = [ "//reverb/cc/platform:logging", "//reverb/cc/platform:snappy", - # "//third_party/absl/status", - # "//third_party/absl/status:statusor", - # "//third_party/absl/strings", - # "//third_party/tensorflow/tsl/platform:status", ] + reverb_tf_deps(), ) @@ -242,8 +235,6 @@ reverb_cc_library( "//reverb/cc/support:signature", "//reverb/cc/support:tf_util", "//reverb/cc/support:trajectory_util", - # "//third_party/absl/log:check", - # "//third_party/grpc:grpc++", ] + reverb_tf_deps() + reverb_grpc_deps() + reverb_absl_deps(), ) diff --git a/reverb/cc/tensor_compression_test.cc b/reverb/cc/tensor_compression_test.cc index 189e4c0..e76523f 100644 --- a/reverb/cc/tensor_compression_test.cc +++ b/reverb/cc/tensor_compression_test.cc @@ -36,7 +36,6 @@ namespace reverb { namespace { using ::testing::HasSubstr; -// using ::testing::status::StatusIs; MATCHER_P2(StatusIs, code, message, "") { return arg.code() == code && absl::StrContains(arg.message(), message); diff --git a/reverb/cc/testing/BUILD b/reverb/cc/testing/BUILD index 21ba89e..a27ecf0 100644 --- a/reverb/cc/testing/BUILD +++ b/reverb/cc/testing/BUILD @@ -18,7 +18,6 @@ reverb_cc_library( "//reverb/cc:schema_cc_proto", "//reverb/cc:tensor_compression", "//reverb/cc/platform:logging", - # "//third_party/absl/log:check", ] + reverb_tf_deps(), ) diff --git a/reverb/cc/writer.cc b/reverb/cc/writer.cc index 1ba5f04..eba7c2a 100644 --- a/reverb/cc/writer.cc +++ b/reverb/cc/writer.cc @@ -34,8 +34,6 @@ #include "absl/synchronization/mutex.h" #include "absl/types/optional.h" #include "grpcpp/impl/codegen/client_context.h" -// #include "third_party/grpc/include/grpcpp/client_context.h" -// #include "third_party/grpc/include/grpcpp/impl/call_op_set.h" #include "reverb/cc/platform/hash_set.h" #include "reverb/cc/platform/logging.h" #include "reverb/cc/platform/status_macros.h"