From 03759cbc5f9a9943b95d51e9888ab16f54f8061a Mon Sep 17 00:00:00 2001 From: Marcel Koch Date: Wed, 31 Jul 2024 10:10:00 +0200 Subject: [PATCH 1/4] [core] allow naming lambda operations --- include/ginkgo/core/base/executor.hpp | 29 +++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/include/ginkgo/core/base/executor.hpp b/include/ginkgo/core/base/executor.hpp index 95373b3e847..8afac213303 100644 --- a/include/ginkgo/core/base/executor.hpp +++ b/include/ginkgo/core/base/executor.hpp @@ -659,6 +659,17 @@ class Executor : public log::EnableLogging { this->run(op); } + template + void run(std::string name, const ClosureOmp& op_omp, + const ClosureCuda& op_cuda, const ClosureHip& op_hip, + const ClosureDpcpp& op_dpcpp) const + { + LambdaOperation op( + std::move(name), op_omp, op_cuda, op_hip, op_dpcpp); + this->run(op); + } + /** * Allocates memory in this Executor. * @@ -1109,6 +1120,16 @@ class Executor : public log::EnableLogging { typename ClosureDpcpp> class LambdaOperation : public Operation { public: + LambdaOperation(std::string name, const ClosureOmp& op_omp, + const ClosureCuda& op_cuda, const ClosureHip& op_hip, + const ClosureDpcpp& op_dpcpp) + : name_(std::move(name)), + op_omp_(op_omp), + op_cuda_(op_cuda), + op_hip_(op_hip), + op_dpcpp_(op_dpcpp) + {} + /** * Creates an LambdaOperation object from four functors. * @@ -1121,10 +1142,7 @@ class Executor : public log::EnableLogging { */ LambdaOperation(const ClosureOmp& op_omp, const ClosureCuda& op_cuda, const ClosureHip& op_hip, const ClosureDpcpp& op_dpcpp) - : op_omp_(op_omp), - op_cuda_(op_cuda), - op_hip_(op_hip), - op_dpcpp_(op_dpcpp) + : LambdaOperation("unnamed", op_omp, op_cuda, op_hip, op_dpcpp) {} void run(std::shared_ptr) const override @@ -1152,7 +1170,10 @@ class Executor : public log::EnableLogging { op_dpcpp_(); } + const char* get_name() const noexcept override { return name_.c_str(); } + private: + std::string name_; ClosureOmp op_omp_; ClosureCuda op_cuda_; ClosureHip op_hip_; From 0d1c677826c088c2a148ce59f8e5ccaad289e901 Mon Sep 17 00:00:00 2001 From: Marcel Koch Date: Thu, 15 Aug 2024 11:42:34 +0200 Subject: [PATCH 2/4] [core] make run(lambda operation) available on all executors --- include/ginkgo/core/base/executor.hpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/include/ginkgo/core/base/executor.hpp b/include/ginkgo/core/base/executor.hpp index 8afac213303..0e338f42044 100644 --- a/include/ginkgo/core/base/executor.hpp +++ b/include/ginkgo/core/base/executor.hpp @@ -1251,8 +1251,6 @@ class ExecutorBase : public Executor { friend class ReferenceExecutor; public: - using Executor::run; - void run(const Operation& op) const override { this->template log(this, &op); @@ -1362,6 +1360,8 @@ class OmpExecutor : public detail::ExecutorBase, friend class detail::ExecutorBase; public: + using Executor::run; + /** * Creates a new OmpExecutor. */ @@ -1439,6 +1439,8 @@ using DefaultExecutor = OmpExecutor; */ class ReferenceExecutor : public OmpExecutor { public: + using Executor::run; + static std::shared_ptr create( std::shared_ptr alloc = std::make_shared()) @@ -1513,6 +1515,8 @@ class CudaExecutor : public detail::ExecutorBase, friend class detail::ExecutorBase; public: + using Executor::run; + /** * Creates a new CudaExecutor. * @@ -1748,6 +1752,8 @@ class HipExecutor : public detail::ExecutorBase, friend class detail::ExecutorBase; public: + using Executor::run; + /** * Creates a new HipExecutor. * @@ -1963,6 +1969,8 @@ class DpcppExecutor : public detail::ExecutorBase, friend class detail::ExecutorBase; public: + using Executor::run; + /** * Creates a new DpcppExecutor. * From 4ceb9e27afd23a512facbde08bd9115aac0fe62a Mon Sep 17 00:00:00 2001 From: Marcel Koch Date: Thu, 15 Aug 2024 11:51:08 +0200 Subject: [PATCH 3/4] [core] add tests for lambda op name --- core/test/base/executor.cpp | 38 +++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/core/test/base/executor.cpp b/core/test/base/executor.cpp index 64a11929983..20f795b2ded 100644 --- a/core/test/base/executor.cpp +++ b/core/test/base/executor.cpp @@ -521,4 +521,42 @@ TEST_F(ExecutorLogging, LogsOperation) } +struct NameLogger : public gko::log::Logger { +protected: + void on_operation_launched(const gko::Executor* exec, + const gko::Operation* op) const override + { + op_name = op->get_name(); + } + +public: + mutable std::string op_name; +}; + + +TEST(LambdaOperation, CanSetName) +{ + auto name_logger = std::make_shared(); + auto exec = gko::ReferenceExecutor::create(); + exec->add_logger(name_logger); + + exec->run( + "name", [] {}, [] {}, [] {}, [] {}); + + ASSERT_EQ("name", name_logger->op_name); +} + + +TEST(LambdaOperation, HasDefaultName) +{ + auto name_logger = std::make_shared(); + auto exec = gko::ReferenceExecutor::create(); + exec->add_logger(name_logger); + + exec->run([] {}, [] {}, [] {}, [] {}); + + ASSERT_EQ("unname", name_logger->op_name); +} + + } // namespace From ac8bbdd72f64b1e00bc768015421fec0f20f128a Mon Sep 17 00:00:00 2001 From: Marcel Koch Date: Mon, 19 Aug 2024 11:38:19 +0200 Subject: [PATCH 4/4] [core] review updates: - test only for existence of default name - add closure for reference op - deprecate lambda run without name Co-authored-by: Pratik Nayak Co-authored-by: Tobias Ribizel --- core/test/base/executor.cpp | 10 ++++- include/ginkgo/core/base/executor.hpp | 55 +++++++++++++++++++-------- test/base/executor.cpp | 22 +++++++++++ 3 files changed, 69 insertions(+), 18 deletions(-) diff --git a/core/test/base/executor.cpp b/core/test/base/executor.cpp index 20f795b2ded..ae037e075df 100644 --- a/core/test/base/executor.cpp +++ b/core/test/base/executor.cpp @@ -541,12 +541,15 @@ TEST(LambdaOperation, CanSetName) exec->add_logger(name_logger); exec->run( - "name", [] {}, [] {}, [] {}, [] {}); + "name", [] {}, [] {}, [] {}, [] {}, [] {}); ASSERT_EQ("name", name_logger->op_name); } +GKO_BEGIN_DISABLE_DEPRECATION_WARNINGS + + TEST(LambdaOperation, HasDefaultName) { auto name_logger = std::make_shared(); @@ -555,8 +558,11 @@ TEST(LambdaOperation, HasDefaultName) exec->run([] {}, [] {}, [] {}, [] {}); - ASSERT_EQ("unname", name_logger->op_name); + ASSERT_NE(nullptr, name_logger->op_name.c_str()); } +GKO_END_DISABLE_DEPRECATION_WARNINGS + + } // namespace diff --git a/include/ginkgo/core/base/executor.hpp b/include/ginkgo/core/base/executor.hpp index 0e338f42044..963e30bfddd 100644 --- a/include/ginkgo/core/base/executor.hpp +++ b/include/ginkgo/core/base/executor.hpp @@ -651,22 +651,42 @@ class Executor : public log::EnableLogging { */ template + GKO_DEPRECATED( + "Please use the overload with std::string as first parameter.") void run(const ClosureOmp& op_omp, const ClosureCuda& op_cuda, const ClosureHip& op_hip, const ClosureDpcpp& op_dpcpp) const { - LambdaOperation op( - op_omp, op_cuda, op_hip, op_dpcpp); + LambdaOperation + op(op_omp, op_cuda, op_hip, op_dpcpp); this->run(op); } - template - void run(std::string name, const ClosureOmp& op_omp, - const ClosureCuda& op_cuda, const ClosureHip& op_hip, - const ClosureDpcpp& op_dpcpp) const + /** + * Runs one of the passed in functors, depending on the Executor type. + * + * @tparam ClosureReference type of op_ref + * @tparam ClosureOmp type of op_omp + * @tparam ClosureCuda type of op_cuda + * @tparam ClosureHip type of op_hip + * @tparam ClosureDpcpp type of op_dpcpp + * + * @param name the name of the operation + * @param op_ref functor to run in case of a ReferenceExecutor + * @param op_omp functor to run in case of a OmpExecutor + * @param op_cuda functor to run in case of a CudaExecutor + * @param op_hip functor to run in case of a HipExecutor + * @param op_dpcpp functor to run in case of a DpcppExecutor + */ + template + void run(std::string name, const ClosureReference& op_ref, + const ClosureOmp& op_omp, const ClosureCuda& op_cuda, + const ClosureHip& op_hip, const ClosureDpcpp& op_dpcpp) const { - LambdaOperation op( - std::move(name), op_omp, op_cuda, op_hip, op_dpcpp); + LambdaOperation + op(std::move(name), op_ref, op_omp, op_cuda, op_hip, op_dpcpp); this->run(op); } @@ -1116,14 +1136,15 @@ class Executor : public log::EnableLogging { * @tparam ClosureHip the type of the third functor * @tparam ClosureDpcpp the type of the fourth functor */ - template + template class LambdaOperation : public Operation { public: - LambdaOperation(std::string name, const ClosureOmp& op_omp, - const ClosureCuda& op_cuda, const ClosureHip& op_hip, - const ClosureDpcpp& op_dpcpp) + LambdaOperation(std::string name, const ClosureReference& op_ref, + const ClosureOmp& op_omp, const ClosureCuda& op_cuda, + const ClosureHip& op_hip, const ClosureDpcpp& op_dpcpp) : name_(std::move(name)), + op_ref_(op_ref), op_omp_(op_omp), op_cuda_(op_cuda), op_hip_(op_hip), @@ -1142,7 +1163,8 @@ class Executor : public log::EnableLogging { */ LambdaOperation(const ClosureOmp& op_omp, const ClosureCuda& op_cuda, const ClosureHip& op_hip, const ClosureDpcpp& op_dpcpp) - : LambdaOperation("unnamed", op_omp, op_cuda, op_hip, op_dpcpp) + : LambdaOperation("unnamed", op_omp, op_omp, op_cuda, op_hip, + op_dpcpp) {} void run(std::shared_ptr) const override @@ -1152,7 +1174,7 @@ class Executor : public log::EnableLogging { void run(std::shared_ptr) const override { - op_omp_(); + op_ref_(); } void run(std::shared_ptr) const override @@ -1174,6 +1196,7 @@ class Executor : public log::EnableLogging { private: std::string name_; + ClosureReference op_ref_; ClosureOmp op_omp_; ClosureCuda op_cuda_; ClosureHip op_hip_; diff --git a/test/base/executor.cpp b/test/base/executor.cpp index 8a344eb224d..7fcab4e0784 100644 --- a/test/base/executor.cpp +++ b/test/base/executor.cpp @@ -90,9 +90,28 @@ TEST_F(Executor, RunsCorrectHostOperation) } +TEST_F(Executor, RunsCorrectLambdaOperationWithReferenceExecutor) +{ + int value = 0; + auto ref_lambda = [&value]() { value = reference::value; }; + auto omp_lambda = [&value]() { value = omp::value; }; + auto cuda_lambda = [&value]() { value = cuda::value; }; + auto hip_lambda = [&value]() { value = hip::value; }; + auto dpcpp_lambda = [&value]() { value = dpcpp::value; }; + + exec->run("test", ref_lambda, omp_lambda, cuda_lambda, hip_lambda, + dpcpp_lambda); + + ASSERT_EQ(GKO_DEVICE_NAMESPACE::value, value); +} + + #ifndef GKO_COMPILING_REFERENCE +GKO_BEGIN_DISABLE_DEPRECATION_WARNINGS + + TEST_F(Executor, RunsCorrectLambdaOperation) { int value = 0; @@ -107,4 +126,7 @@ TEST_F(Executor, RunsCorrectLambdaOperation) } +GKO_END_DISABLE_DEPRECATION_WARNINGS + + #endif // GKO_COMPILING_REFERENCE