From d8e5c05e10445b1dc32d31158fdd294208be3176 Mon Sep 17 00:00:00 2001 From: Gerhard Raven <gerhard.raven@nikhef.nl> Date: Thu, 15 Apr 2021 09:00:36 +0200 Subject: [PATCH 1/3] Allow returning void in a Merging transformer The current MergingTransformer signature cannot return void. This is fixing it --- GaudiAlg/include/GaudiAlg/FunctionalDetails.h | 9 +++++ .../include/GaudiAlg/MergingTransformer.h | 35 ++++++++++++++++--- .../FunctionalAlgorithms/MakeAndConsume.cpp | 24 ++++++++++++- 3 files changed, 63 insertions(+), 5 deletions(-) diff --git a/GaudiAlg/include/GaudiAlg/FunctionalDetails.h b/GaudiAlg/include/GaudiAlg/FunctionalDetails.h index 7de44f2394..c8ee0728b4 100644 --- a/GaudiAlg/include/GaudiAlg/FunctionalDetails.h +++ b/GaudiAlg/include/GaudiAlg/FunctionalDetails.h @@ -536,6 +536,8 @@ namespace Gaudi::Functional::details { static_assert( std::is_base_of_v<Algorithm, BaseClass_t<Traits_>>, "BaseClass must inherit from Algorithm" ); public: + using KeyValue = std::pair<std::string, std::string>; + using KeyValues = std::pair<std::string, std::vector<std::string>>; DataHandleMixin( std::string name, ISvcLocator* pSvcLocator ) : BaseClass_t<Traits_>( std::move( name ), pSvcLocator ) { // make sure this algorithm is seen as reentrant by Gaudi @@ -590,6 +592,13 @@ namespace Gaudi::Functional::details { std::tuple<details::InputHandle_t<Traits_, In>...> m_inputs; }; + template <typename Traits_> + class DataHandleMixin<std::tuple<void>, std::tuple<>, Traits_> + : public DataHandleMixin<std::tuple<>, std::tuple<>, Traits_> { + public: + using DataHandleMixin<std::tuple<>, std::tuple<>, Traits_>::DataHandleMixin; + }; + template <typename... Out, typename Traits_> class DataHandleMixin<std::tuple<Out...>, std::tuple<>, Traits_> : public BaseClass_t<Traits_> { static_assert( std::is_base_of_v<Algorithm, BaseClass_t<Traits_>>, "BaseClass must inherit from Algorithm" ); diff --git a/GaudiAlg/include/GaudiAlg/MergingTransformer.h b/GaudiAlg/include/GaudiAlg/MergingTransformer.h index 7a5c780ae0..c605f92100 100644 --- a/GaudiAlg/include/GaudiAlg/MergingTransformer.h +++ b/GaudiAlg/include/GaudiAlg/MergingTransformer.h @@ -29,7 +29,7 @@ namespace Gaudi::Functional { template <typename Signature, typename Traits_, bool isLegacy> struct MergingTransformer; - ////// Many of the same -> 1 + ////// Many of the same -> 1 or 0 template <typename Out, typename In, typename Traits_> struct MergingTransformer<Out( const vector_of_const_<In>& ), Traits_, true> : DataHandleMixin<std::tuple<Out>, std::tuple<>, Traits_> { @@ -65,7 +65,11 @@ namespace Gaudi::Functional { ins.reserve( m_inputs.size() ); std::transform( m_inputs.begin(), m_inputs.end(), std::back_inserter( ins ), details2::get_from_handle<In>{} ); try { - put( std::get<0>( this->m_outputs ), std::as_const( *this )( std::as_const( ins ) ) ); + if constexpr ( std::is_void_v<Out> ) { + std::as_const ( *this )( std::as_const( ins ) ); + } else { + put( std::get<0>( this->m_outputs ), std::as_const( *this )( std::as_const( ins ) ) ); + } return FilterDecision::PASSED; } catch ( GaudiException& e ) { ( e.code() ? this->warning() : this->error() ) << e.message() << endmsg; @@ -95,6 +99,23 @@ namespace Gaudi::Functional { using KeyValue = typename base_class::KeyValue; using KeyValues = typename base_class::KeyValues; + MergingTransformer( std::string name, ISvcLocator* locator, const KeyValues& inputs ) + : base_class( std::move( name ), locator ) + , m_inputLocations{this, inputs.first, inputs.second, + [=]( Gaudi::Details::PropertyBase& ) { + this->m_inputs = + make_vector_of_handles<decltype( this->m_inputs )>( this, m_inputLocations ); + if ( std::is_pointer_v<In> ) { // handle constructor does not (yet) allow to set + // optional flag... so do it + // explicitly here... + std::for_each( this->m_inputs.begin(), this->m_inputs.end(), + []( auto& h ) { h.setOptional( true ); } ); + } + }, + Gaudi::Details::Property::ImmediatelyInvokeHandler{true}} { + static_assert( std::is_void_v<Out> ); + } + MergingTransformer( std::string name, ISvcLocator* locator, const KeyValues& inputs, const KeyValue& output ) : base_class( std::move( name ), locator, output ) , m_inputLocations{this, inputs.first, inputs.second, @@ -108,7 +129,9 @@ namespace Gaudi::Functional { []( auto& h ) { h.setOptional( true ); } ); } }, - Gaudi::Details::Property::ImmediatelyInvokeHandler{true}} {} + Gaudi::Details::Property::ImmediatelyInvokeHandler{true}} { + static_assert( !std::is_void_v<Out> ); + } // accessor to input Locations const std::string& inputLocation( unsigned int n ) const { return m_inputLocations.value()[n]; } @@ -120,7 +143,11 @@ namespace Gaudi::Functional { ins.reserve( m_inputs.size() ); std::transform( m_inputs.begin(), m_inputs.end(), std::back_inserter( ins ), details2::get_from_handle<In>{} ); try { - put( std::get<0>( this->m_outputs ), ( *this )( std::as_const( ins ) ) ); + if constexpr ( std::is_void_v<Out> ) { + ( *this )( std::as_const( ins ) ); + } else { + put( std::get<0>( this->m_outputs ), ( *this )( std::as_const( ins ) ) ); + } return FilterDecision::PASSED; } catch ( GaudiException& e ) { ( e.code() ? this->warning() : this->error() ) << e.message() << endmsg; diff --git a/GaudiExamples/src/FunctionalAlgorithms/MakeAndConsume.cpp b/GaudiExamples/src/FunctionalAlgorithms/MakeAndConsume.cpp index 461a3f147b..0f8ec7e524 100644 --- a/GaudiExamples/src/FunctionalAlgorithms/MakeAndConsume.cpp +++ b/GaudiExamples/src/FunctionalAlgorithms/MakeAndConsume.cpp @@ -386,7 +386,29 @@ namespace Gaudi::Examples { return out; } }; - DECLARE_COMPONENT( SRangesToIntVector ) + + struct IntVectorsMerger final + : public Gaudi::Functional::MergingTransformer< + void( const Gaudi::Functional::vector_of_const_<std::vector<int>>& ), BaseClass_t> { + + IntVectorsMerger( const std::string& name, ISvcLocator* svcLoc ) + : MergingTransformer( name, svcLoc, {"InputLocations", {}}) {} + + void + operator()( const Gaudi::Functional::vector_of_const_<std::vector<int>>& intVectors ) const override { + // Create a vector and pre-allocate enough space for the number of integers we have + auto nelements = std::accumulate( intVectors.begin(), intVectors.end(), 0, + []( const auto a, const auto b ) { return a + b.size(); } ); + info() << "sum of input sizes: " << nelements << endmsg; + // Concatenate the input vectors to form the output + for ( const auto& intVector : intVectors ) { + info() << "Consuming vector " << intVector << endmsg; + } + } + }; + + DECLARE_COMPONENT( IntVectorsMerger ) + } // namespace Gaudi::Examples -- GitLab From f26f62f64089bbb89a07c007b8e3f4e0773f29e3 Mon Sep 17 00:00:00 2001 From: Sebastien Ponce <sebastien.ponce@cern.ch> Date: Thu, 15 Apr 2021 10:05:10 +0200 Subject: [PATCH 2/3] Introduced MergingConsumer alias in functional Algorithms This is purely a convenience alias to MergingTransformer created for consistency with the naming of the Consumer algorithms. Also added tests for it, as well as the missing python code for testing the MergingTransformer with void output --- .../include/GaudiAlg/MergingTransformer.h | 12 +++++++ .../FunctionalAlgorithms/ProduceConsume.py | 14 ++++++++ .../FunctionalAlgorithms/MakeAndConsume.cpp | 36 ++++++++++++++----- .../tests/qmtest/refs/ProduceConsume.ref | 21 ++++++++--- 4 files changed, 70 insertions(+), 13 deletions(-) diff --git a/GaudiAlg/include/GaudiAlg/MergingTransformer.h b/GaudiAlg/include/GaudiAlg/MergingTransformer.h index c605f92100..ca42cb680d 100644 --- a/GaudiAlg/include/GaudiAlg/MergingTransformer.h +++ b/GaudiAlg/include/GaudiAlg/MergingTransformer.h @@ -26,6 +26,13 @@ namespace Gaudi::Functional { namespace details { + template <typename Sig> + struct is_void_fun : std::false_type {}; + template <typename... Args> + struct is_void_fun<void( Args... )> : std::true_type {}; + template <typename Sig> + inline constexpr bool is_void_fun_v = is_void_fun<Sig>::value; + template <typename Signature, typename Traits_, bool isLegacy> struct MergingTransformer; @@ -172,6 +179,11 @@ namespace Gaudi::Functional { template <typename Signature, typename Traits_ = Traits::useDefaults> using MergingTransformer = details::MergingTransformer<Signature, Traits_, details::isLegacy<Traits_>>; + // more meaningful alias for cases where the return type in Signature is void + template <typename Signature, typename Traits_ = Traits::useDefaults, + typename = std::enable_if_t<details::is_void_fun_v<Signature>>> + using MergingConsumer = details::MergingTransformer<Signature, Traits_, details::isLegacy<Traits_>>; + // Many of the same -> N template <typename Signature, typename Traits_ = Traits::BaseClass_t<Gaudi::Algorithm>> struct MergingMultiTransformer; diff --git a/GaudiExamples/options/FunctionalAlgorithms/ProduceConsume.py b/GaudiExamples/options/FunctionalAlgorithms/ProduceConsume.py index 50d22c4052..c6705e3bed 100644 --- a/GaudiExamples/options/FunctionalAlgorithms/ProduceConsume.py +++ b/GaudiExamples/options/FunctionalAlgorithms/ProduceConsume.py @@ -21,6 +21,8 @@ from Configurables import Gaudi__Examples__IntToFloatData as IntToFloatData from Configurables import Gaudi__Examples__IntIntToFloatFloatData as IntIntToFloatFloatData from Configurables import Gaudi__Examples__IntVectorsToIntVector as IntVectorsToIntVector from Configurables import Gaudi__Examples__SRangesToIntVector as SRangesToIntVector +from Configurables import Gaudi__Examples__IntVectorsMerger as IntVectorsMerger +from Configurables import Gaudi__Examples__IntVectorsMergingConsumer as IntVectorsMergingConsumer from Configurables import Gaudi__Examples__ContextConsumer as ContextConsumer from Configurables import Gaudi__Examples__ContextIntConsumer as ContextIntConsumer from Configurables import Gaudi__Examples__VectorDoubleProducer as VectorDoubleProducer @@ -98,6 +100,18 @@ app.TopAlg = [ str(SDataProducer1.OutputLocation), str(SDataProducer2.OutputLocation) ]), + IntVectorsMerger( + "IntVectorsMerger", + InputLocations=[ + str(VectorDataProducer1.OutputLocation), + str(VectorDataProducer2.OutputLocation) + ]), + IntVectorsMergingConsumer( + "IntVectorsMergingConsumer", + InputLocations=[ + str(VectorDataProducer1.OutputLocation), + str(VectorDataProducer2.OutputLocation) + ]), ] # - Events app.EvtMax = 2 diff --git a/GaudiExamples/src/FunctionalAlgorithms/MakeAndConsume.cpp b/GaudiExamples/src/FunctionalAlgorithms/MakeAndConsume.cpp index 0f8ec7e524..fe9b1c4ccd 100644 --- a/GaudiExamples/src/FunctionalAlgorithms/MakeAndConsume.cpp +++ b/GaudiExamples/src/FunctionalAlgorithms/MakeAndConsume.cpp @@ -388,27 +388,45 @@ namespace Gaudi::Examples { }; DECLARE_COMPONENT( SRangesToIntVector ) - - struct IntVectorsMerger final + struct IntVectorsMerger final : public Gaudi::Functional::MergingTransformer< void( const Gaudi::Functional::vector_of_const_<std::vector<int>>& ), BaseClass_t> { IntVectorsMerger( const std::string& name, ISvcLocator* svcLoc ) - : MergingTransformer( name, svcLoc, {"InputLocations", {}}) {} + : MergingTransformer( name, svcLoc, {"InputLocations", {}} ) {} - void - operator()( const Gaudi::Functional::vector_of_const_<std::vector<int>>& intVectors ) const override { + void operator()( const Gaudi::Functional::vector_of_const_<std::vector<int>>& intVectors ) const override { // Create a vector and pre-allocate enough space for the number of integers we have - auto nelements = std::accumulate( intVectors.begin(), intVectors.end(), 0, + auto nelements = std::accumulate( intVectors.begin(), intVectors.end(), 0, []( const auto a, const auto b ) { return a + b.size(); } ); info() << "sum of input sizes: " << nelements << endmsg; // Concatenate the input vectors to form the output - for ( const auto& intVector : intVectors ) { - info() << "Consuming vector " << intVector << endmsg; - } + for ( const auto& intVector : intVectors ) { info() << "Consuming vector " << intVector << endmsg; } } }; DECLARE_COMPONENT( IntVectorsMerger ) + struct IntVectorsMergingConsumer final + : public Gaudi::Functional::MergingConsumer<void( Gaudi::Functional::vector_of_const_<std::vector<int>> const& ), + BaseClass_t> { + using Base = + Gaudi::Functional::MergingConsumer<void( Gaudi::Functional::vector_of_const_<std::vector<int>> const& ), + BaseClass_t>; + + IntVectorsMergingConsumer( const std::string& name, ISvcLocator* svcLoc ) + : Base( name, svcLoc, {"InputLocations", {}} ) {} + + void operator()( Gaudi::Functional::vector_of_const_<std::vector<int>> const& intVectors ) const override { + // Create a vector and pre-allocate enough space for the number of integers we have + auto nelements = std::accumulate( intVectors.begin(), intVectors.end(), 0, + []( const auto a, const auto b ) { return a + b.size(); } ); + info() << "sum of input sizes: " << nelements << endmsg; + // Concatenate the input vectors to form the output + for ( const auto& intVector : intVectors ) { info() << "Consuming vector " << intVector << endmsg; } + } + }; + + DECLARE_COMPONENT( IntVectorsMergingConsumer ) + } // namespace Gaudi::Examples diff --git a/GaudiExamples/tests/qmtest/refs/ProduceConsume.ref b/GaudiExamples/tests/qmtest/refs/ProduceConsume.ref index 508b101ef2..6deac60519 100644 --- a/GaudiExamples/tests/qmtest/refs/ProduceConsume.ref +++ b/GaudiExamples/tests/qmtest/refs/ProduceConsume.ref @@ -1,4 +1,5 @@ -# --> Including file '/home/nnolte/lb-stack-setup/stack2/Gaudi/GaudiExamples/options/FunctionalAlgorithms/ProduceConsume.py' +# setting LC_ALL to "C" +# --> Including file '/home/sponce/dev4/Gaudi/GaudiExamples/options/FunctionalAlgorithms/ProduceConsume.py' --- # List of input and output types by class "Gaudi::Examples::ContextConsumer": @@ -42,11 +43,11 @@ "Gaudi::Examples::VectorDoubleProducer": OutputLocation: "std::vector<double,std::allocator<double> >" --- -# <-- End of file '/home/nnolte/lb-stack-setup/stack2/Gaudi/GaudiExamples/options/FunctionalAlgorithms/ProduceConsume.py' +# <-- End of file '/home/sponce/dev4/Gaudi/GaudiExamples/options/FunctionalAlgorithms/ProduceConsume.py' ApplicationMgr SUCCESS ==================================================================================================================================== - Welcome to ApplicationMgr (GaudiCoreSvc v33r0) - running on lbquantaperf01 on Thu Mar 19 13:47:12 2020 + Welcome to ApplicationMgr (GaudiCoreSvc v35r2) + running on lblhcbpr11.cern.ch on Thu Apr 15 10:03:11 2021 ==================================================================================================================================== ApplicationMgr INFO Application Manager Configured successfully EventLoopMgr WARNING Unable to locate service "EventSelector" @@ -102,6 +103,12 @@ SDataProducer2 INFO storing KeyedContainer of size 10 into /Event/S2 SRangesToIntVector INFO Concatening range of size 3 SRangesToIntVector INFO Concatening range of size 10 SRangesToIntVector INFO Storing output vector [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] to /Event/MyConcatenatedIntFromSVector +IntVectorsMerger INFO sum of input sizes: 8 +IntVectorsMerger INFO Consuming vector [3, 3, 3, 3] +IntVectorsMerger INFO Consuming vector [3, 3, 3, 3] +IntVectorsMergi... INFO sum of input sizes: 8 +IntVectorsMergi... INFO Consuming vector [3, 3, 3, 3] +IntVectorsMergi... INFO Consuming vector [3, 3, 3, 3] IntDataProducer INFO executing IntDataProducer, storing 7 into /Event/MyInt OtherIntDataPro... INFO executing IntDataProducer, storing 7 into /Event/MyOtherInt IntDataConsumer INFO executing IntDataConsumer, consuming 7 from /Event/MyInt @@ -147,6 +154,12 @@ SDataProducer2 INFO storing KeyedContainer of size 10 into /Event/S2 SRangesToIntVector INFO Concatening range of size 3 SRangesToIntVector INFO Concatening range of size 10 SRangesToIntVector INFO Storing output vector [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] to /Event/MyConcatenatedIntFromSVector +IntVectorsMerger INFO sum of input sizes: 8 +IntVectorsMerger INFO Consuming vector [3, 3, 3, 3] +IntVectorsMerger INFO Consuming vector [3, 3, 3, 3] +IntVectorsMergi... INFO sum of input sizes: 8 +IntVectorsMergi... INFO Consuming vector [3, 3, 3, 3] +IntVectorsMergi... INFO Consuming vector [3, 3, 3, 3] CountingConsumer INFO Number of counters : 3 | Counter | # | sum | mean/eff^* | rms/err^* | min | max | | "This is not a warning..." | 4 | -- GitLab From 0e9d2be88e46c07c1bcfc6bc9968dcb9108e2485 Mon Sep 17 00:00:00 2001 From: Sebastien Ponce <sebastien.ponce@cern.ch> Date: Thu, 15 Apr 2021 16:52:13 +0200 Subject: [PATCH 3/3] Added an extra MergingTransformer constructor This one is dedicated to void returning signatures in case of legacy algorithm --- .../include/GaudiAlg/MergingTransformer.h | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/GaudiAlg/include/GaudiAlg/MergingTransformer.h b/GaudiAlg/include/GaudiAlg/MergingTransformer.h index ca42cb680d..6bcc8177eb 100644 --- a/GaudiAlg/include/GaudiAlg/MergingTransformer.h +++ b/GaudiAlg/include/GaudiAlg/MergingTransformer.h @@ -47,6 +47,23 @@ namespace Gaudi::Functional { using KeyValue = typename base_class::KeyValue; using KeyValues = typename base_class::KeyValues; + MergingTransformer( std::string name, ISvcLocator* locator, const KeyValues& inputs ) + : base_class( std::move( name ), locator ) + , m_inputLocations{this, inputs.first, inputs.second, + [=]( Gaudi::Details::PropertyBase& ) { + this->m_inputs = + make_vector_of_handles<decltype( this->m_inputs )>( this, m_inputLocations ); + if ( std::is_pointer_v<In> ) { // handle constructor does not (yet) allow to set + // optional flag... so do it + // explicitly here... + std::for_each( this->m_inputs.begin(), this->m_inputs.end(), + []( auto& h ) { h.setOptional( true ); } ); + } + }, + Gaudi::Details::Property::ImmediatelyInvokeHandler{true}} { + static_assert( std::is_void_v<Out> ); + } + MergingTransformer( std::string name, ISvcLocator* locator, const KeyValues& inputs, const KeyValue& output ) : base_class( std::move( name ), locator, output ) , m_inputLocations{this, inputs.first, inputs.second, @@ -60,7 +77,9 @@ namespace Gaudi::Functional { []( auto& h ) { h.setOptional( true ); } ); } }, - Gaudi::Details::Property::ImmediatelyInvokeHandler{true}} {} + Gaudi::Details::Property::ImmediatelyInvokeHandler{true}} { + static_assert( !std::is_void_v<Out> ); + } // accessor to input Locations const std::string& inputLocation( unsigned int n ) const { return m_inputLocations.value()[n]; } -- GitLab