From 6f60e399f53ceda56e2da56b2372c13a23504ffa Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 9 Dec 2024 19:19:38 +0100 Subject: [PATCH 01/10] Ignore objects that are not collections in Writer instead of returning --- k4FWCore/components/Writer.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/k4FWCore/components/Writer.cpp b/k4FWCore/components/Writer.cpp index 64d02d35..b5da0604 100644 --- a/k4FWCore/components/Writer.cpp +++ b/k4FWCore/components/Writer.cpp @@ -257,8 +257,15 @@ class Writer final : public Gaudi::Functional::Consumer(storeCollection); if (!old_collection) { - error() << "Failed to cast collection " << coll << endmsg; - return; + // This can happen for objects that are not collections like in the + // MarlinWrapper for converter maps or a LCEvent, or, in general, + // anything else + warning() << "Object in the store with name " << coll + << " does not look like a collection so it can not be written to the output file" << endmsg; + m_collectionsToSave.erase(std::remove(m_collectionsToSave.begin(), m_collectionsToSave.end(), coll), + m_collectionsToSave.end()); + m_collectionsToAdd.erase(std::remove(m_collectionsToAdd.begin(), m_collectionsToAdd.end(), coll), + m_collectionsToAdd.end()); } else { std::unique_ptr uptr( const_cast(old_collection->collectionBase())); From 0f9af0f6380e5f9223d6e25f40a1cd3678ad68c9 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 9 Dec 2024 19:23:12 +0100 Subject: [PATCH 02/10] Add a test that uses a Writer with a plain Gaudi Functional algorithm --- test/k4FWCoreTest/CMakeLists.txt | 1 + .../options/ExampleGaudiFunctional.py | 41 +++++++++++++++++++ .../ExampleGaudiFunctionalProducer.cpp | 38 +++++++++++++++++ 3 files changed, 80 insertions(+) create mode 100644 test/k4FWCoreTest/options/ExampleGaudiFunctional.py create mode 100644 test/k4FWCoreTest/src/components/ExampleGaudiFunctionalProducer.cpp diff --git a/test/k4FWCoreTest/CMakeLists.txt b/test/k4FWCoreTest/CMakeLists.txt index 9340b296..3c66fec6 100644 --- a/test/k4FWCoreTest/CMakeLists.txt +++ b/test/k4FWCoreTest/CMakeLists.txt @@ -188,6 +188,7 @@ set_tests_properties(FunctionalWrongImport PROPERTIES PASS_REGULAR_EXPRESSION "I add_test_with_env(FunctionalReadNthEvent options/ExampleFunctionalReadNthEvent.py PROPERTIES DEPENDS FunctionalProducer ADD_TO_CHECK_FILES) add_test_with_env(FunctionalProducerRNTuple options/ExampleFunctionalProducerRNTuple.py ADD_TO_CHECK_FILES) add_test_with_env(FunctionalTTreeToRNTuple options/ExampleFunctionalTTreeToRNTuple.py PROPERTIES DEPENDS FunctionalProducer ADD_TO_CHECK_FILES) +add_test_with_env(GaudiFunctional options/ExampleGaudiFunctional.py) # The following is done to make the tests work without installing the files in diff --git a/test/k4FWCoreTest/options/ExampleGaudiFunctional.py b/test/k4FWCoreTest/options/ExampleGaudiFunctional.py new file mode 100644 index 00000000..0162f80d --- /dev/null +++ b/test/k4FWCoreTest/options/ExampleGaudiFunctional.py @@ -0,0 +1,41 @@ +# +# Copyright (c) 2014-2024 Key4hep-Project. +# +# This file is part of Key4hep. +# See https://key4hep.github.io/key4hep-doc/ for further info. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +# This is an example reading from a file and using a producer to create new +# data + +from Gaudi.Configuration import INFO +from Configurables import ExampleGaudiFunctionalProducer +from Configurables import EventDataSvc +from k4FWCore import ApplicationMgr, IOSvc + +svc = IOSvc("IOSvc") +svc.Output = "functional_transformer.root" + +producer = ExampleGaudiFunctionalProducer( + "Producer", OutputCollectionName="Output" +) + +mgr = ApplicationMgr( + TopAlg=[producer], + EvtSel="NONE", + EvtMax=3, + ExtSvc=[EventDataSvc("EventDataSvc")], + OutputLevel=INFO, +) diff --git a/test/k4FWCoreTest/src/components/ExampleGaudiFunctionalProducer.cpp b/test/k4FWCoreTest/src/components/ExampleGaudiFunctionalProducer.cpp new file mode 100644 index 00000000..171036d4 --- /dev/null +++ b/test/k4FWCoreTest/src/components/ExampleGaudiFunctionalProducer.cpp @@ -0,0 +1,38 @@ +/* + * Copyright (c) 2014-2024 Key4hep-Project. + * + * This file is part of Key4hep. + * See https://key4hep.github.io/key4hep-doc/ for further info. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "Gaudi/Functional/Producer.h" + +#include + +using BaseClass_t = Gaudi::Functional::Traits::BaseClass_t; + +struct ExampleGaudiFunctionalProducer final : Gaudi::Functional::Producer { + // The pair in KeyValues can be changed from python and it corresponds + // to the name of the output collection + ExampleGaudiFunctionalProducer(const std::string& name, ISvcLocator* svcLoc) + : Producer(name, svcLoc, KeyValue{"OutputCollectionName", "OutputCollection"}) {} + + // This is the function that will be called to produce the data + int operator()() const override { + return 3; + } +}; + +DECLARE_COMPONENT(ExampleGaudiFunctionalProducer) From 535c7ae17ec8b099a6431b2ccb859b8e6a23f919 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 9 Dec 2024 19:24:03 +0100 Subject: [PATCH 03/10] Add const when possible in the writer and add [[maybe_unused]] for released pointer values --- k4FWCore/components/Writer.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/k4FWCore/components/Writer.cpp b/k4FWCore/components/Writer.cpp index b5da0604..cb9d6762 100644 --- a/k4FWCore/components/Writer.cpp +++ b/k4FWCore/components/Writer.cpp @@ -156,7 +156,7 @@ class Writer final : public Gaudi::Functional::Consumername() == k4FWCore::frameLocation) { continue; } @@ -190,7 +190,7 @@ class Writer final : public Gaudi::Functional::ConsumerunregisterObject(p); + const auto sc = m_dataSvc->unregisterObject(p); if (!sc.isSuccess()) { error() << "Failed to unregister object" << endmsg; return; @@ -221,7 +221,7 @@ class Writer final : public Gaudi::Functional::ConsumerretrieveObject("/Event/" + coll, storeCollection).isFailure()) { error() << "Failed to retrieve collection " << coll << endmsg; @@ -233,12 +233,12 @@ class Writer final : public Gaudi::Functional::Consumer>*>(storeCollection); - storePtr->getData().release(); + const auto storePtr = dynamic_cast>*>(storeCollection); + [[maybe_unused]] auto releasedPtr = storePtr->getData().release(); delete storePtr; } - for (auto& coll : m_collectionsToAdd) { + for (const auto& coll : m_collectionsToAdd) { DataObject* storeCollection; if (m_dataSvc->retrieveObject("/Event/" + coll, storeCollection).isFailure()) { error() << "Failed to retrieve collection " << coll << endmsg; From d31599a285cb455ad2c5f1bed0c3875be9b085a4 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 9 Dec 2024 19:27:49 +0100 Subject: [PATCH 04/10] Fix pre-commit --- test/k4FWCoreTest/options/ExampleGaudiFunctional.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/k4FWCoreTest/options/ExampleGaudiFunctional.py b/test/k4FWCoreTest/options/ExampleGaudiFunctional.py index 0162f80d..1effa838 100644 --- a/test/k4FWCoreTest/options/ExampleGaudiFunctional.py +++ b/test/k4FWCoreTest/options/ExampleGaudiFunctional.py @@ -28,9 +28,7 @@ svc = IOSvc("IOSvc") svc.Output = "functional_transformer.root" -producer = ExampleGaudiFunctionalProducer( - "Producer", OutputCollectionName="Output" -) +producer = ExampleGaudiFunctionalProducer("Producer", OutputCollectionName="Output") mgr = ApplicationMgr( TopAlg=[producer], From e7e85fea5ddafbd725951e40c6198a4e90ec1272 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 9 Dec 2024 21:05:53 +0100 Subject: [PATCH 05/10] Make the test also read and produce some collections --- test/k4FWCoreTest/CMakeLists.txt | 2 +- .../k4FWCoreTest/options/ExampleGaudiFunctional.py | 14 +++++++++----- test/k4FWCoreTest/scripts/CheckOutputFiles.py | 1 + 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/test/k4FWCoreTest/CMakeLists.txt b/test/k4FWCoreTest/CMakeLists.txt index 3c66fec6..0e7291e4 100644 --- a/test/k4FWCoreTest/CMakeLists.txt +++ b/test/k4FWCoreTest/CMakeLists.txt @@ -188,7 +188,7 @@ set_tests_properties(FunctionalWrongImport PROPERTIES PASS_REGULAR_EXPRESSION "I add_test_with_env(FunctionalReadNthEvent options/ExampleFunctionalReadNthEvent.py PROPERTIES DEPENDS FunctionalProducer ADD_TO_CHECK_FILES) add_test_with_env(FunctionalProducerRNTuple options/ExampleFunctionalProducerRNTuple.py ADD_TO_CHECK_FILES) add_test_with_env(FunctionalTTreeToRNTuple options/ExampleFunctionalTTreeToRNTuple.py PROPERTIES DEPENDS FunctionalProducer ADD_TO_CHECK_FILES) -add_test_with_env(GaudiFunctional options/ExampleGaudiFunctional.py) +add_test_with_env(GaudiFunctional options/ExampleGaudiFunctional.py PROPERTIES DEPENDS FunctionalProducer ADD_TO_CHECK_FILES) # The following is done to make the tests work without installing the files in diff --git a/test/k4FWCoreTest/options/ExampleGaudiFunctional.py b/test/k4FWCoreTest/options/ExampleGaudiFunctional.py index 1effa838..ae7ea0b1 100644 --- a/test/k4FWCoreTest/options/ExampleGaudiFunctional.py +++ b/test/k4FWCoreTest/options/ExampleGaudiFunctional.py @@ -21,19 +21,23 @@ # data from Gaudi.Configuration import INFO -from Configurables import ExampleGaudiFunctionalProducer +from Configurables import ExampleGaudiFunctionalProducer, ExampleFunctionalTransformer from Configurables import EventDataSvc from k4FWCore import ApplicationMgr, IOSvc svc = IOSvc("IOSvc") -svc.Output = "functional_transformer.root" +svc.Input = "functional_producer.root" +svc.Output = "gaudi_functional.root" -producer = ExampleGaudiFunctionalProducer("Producer", OutputCollectionName="Output") +gaudi_producer = ExampleGaudiFunctionalProducer("GaudiProducer", OutputCollectionName="Output") +transformer = ExampleFunctionalTransformer( + "Transformer", InputCollection=["MCParticles"], OutputCollection=["NewMCParticles"] +) mgr = ApplicationMgr( - TopAlg=[producer], + TopAlg=[gaudi_producer, transformer], EvtSel="NONE", - EvtMax=3, + EvtMax=-1, ExtSvc=[EventDataSvc("EventDataSvc")], OutputLevel=INFO, ) diff --git a/test/k4FWCoreTest/scripts/CheckOutputFiles.py b/test/k4FWCoreTest/scripts/CheckOutputFiles.py index 67c98238..518eb087 100644 --- a/test/k4FWCoreTest/scripts/CheckOutputFiles.py +++ b/test/k4FWCoreTest/scripts/CheckOutputFiles.py @@ -70,6 +70,7 @@ def check_metadata(filename, expected_metadata): check_collections("functional_transformer.root", ["MCParticles", "NewMCParticles"]) +check_collections("gaudi_functional.root", ["MCParticles", "NewMCParticles"]) check_collections("functional_transformer_cli.root", ["MCParticles", "NewMCParticles"]) check_collections( "functional_transformer_multiple.root", From 8e3e498a22ed055dcc7385d308ec8857a1d460a6 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 9 Dec 2024 21:06:15 +0100 Subject: [PATCH 06/10] Sort includes in Writer --- k4FWCore/components/Writer.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/k4FWCore/components/Writer.cpp b/k4FWCore/components/Writer.cpp index cb9d6762..41b506e9 100644 --- a/k4FWCore/components/Writer.cpp +++ b/k4FWCore/components/Writer.cpp @@ -21,8 +21,10 @@ #include "GaudiKernel/AnyDataWrapper.h" #include "GaudiKernel/IDataManagerSvc.h" #include "GaudiKernel/IDataProviderSvc.h" +#include "GaudiKernel/IHiveWhiteBoard.h" #include "GaudiKernel/SmartDataPtr.h" #include "GaudiKernel/StatusCode.h" + #include "podio/Frame.h" #include "IIOSvc.h" @@ -31,7 +33,7 @@ #include "k4FWCore/FunctionalUtils.h" #include "k4FWCore/IMetadataSvc.h" -#include +#include #include #include From 6f98928e945cfd22d3d4350e5283e7fe586059f4 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 9 Dec 2024 21:08:47 +0100 Subject: [PATCH 07/10] Fix pre-commit --- .../src/components/ExampleGaudiFunctionalProducer.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/k4FWCoreTest/src/components/ExampleGaudiFunctionalProducer.cpp b/test/k4FWCoreTest/src/components/ExampleGaudiFunctionalProducer.cpp index 171036d4..8d1250c4 100644 --- a/test/k4FWCoreTest/src/components/ExampleGaudiFunctionalProducer.cpp +++ b/test/k4FWCoreTest/src/components/ExampleGaudiFunctionalProducer.cpp @@ -27,12 +27,10 @@ struct ExampleGaudiFunctionalProducer final : Gaudi::Functional::Producer Date: Fri, 13 Dec 2024 10:46:47 +0100 Subject: [PATCH 08/10] Change a warning message to info --- k4FWCore/components/Writer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/k4FWCore/components/Writer.cpp b/k4FWCore/components/Writer.cpp index 41b506e9..7ee461c6 100644 --- a/k4FWCore/components/Writer.cpp +++ b/k4FWCore/components/Writer.cpp @@ -262,8 +262,8 @@ class Writer final : public Gaudi::Functional::Consumer Date: Tue, 17 Dec 2024 10:13:56 +0100 Subject: [PATCH 09/10] Fix the issue where the vector being iterated over is modified in the loop --- k4FWCore/components/Writer.cpp | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/k4FWCore/components/Writer.cpp b/k4FWCore/components/Writer.cpp index 7ee461c6..851ea404 100644 --- a/k4FWCore/components/Writer.cpp +++ b/k4FWCore/components/Writer.cpp @@ -19,6 +19,7 @@ #include "Gaudi/Functional/Consumer.h" #include "GaudiKernel/AnyDataWrapper.h" +#include "GaudiKernel/DataObject.h" #include "GaudiKernel/IDataManagerSvc.h" #include "GaudiKernel/IDataProviderSvc.h" #include "GaudiKernel/IHiveWhiteBoard.h" @@ -133,8 +134,8 @@ class Writer final : public Gaudi::Functional::Consumer m_mgr; - m_mgr = eventSvc(); + SmartIF mgr; + mgr = eventSvc(); SmartDataPtr root(eventSvc(), "/Event"); if (!root) { @@ -147,13 +148,8 @@ class Writer final : public Gaudi::Functional::Consumer(); - if (!mgr) { - error() << "Failed to retrieve IDataManagerSvc" << endmsg; - return; - } std::vector leaves; - StatusCode sc = m_mgr->objectLeaves(pObj, leaves); + StatusCode sc = mgr->objectLeaves(pObj, leaves); if (!sc.isSuccess()) { error() << "Failed to retrieve object leaves" << endmsg; return; @@ -240,6 +236,7 @@ class Writer final : public Gaudi::Functional::Consumer collectionsToRemove; for (const auto& coll : m_collectionsToAdd) { DataObject* storeCollection; if (m_dataSvc->retrieveObject("/Event/" + coll, storeCollection).isFailure()) { @@ -264,10 +261,11 @@ class Writer final : public Gaudi::Functional::Consumer uptr( const_cast(old_collection->collectionBase())); @@ -276,6 +274,11 @@ class Writer final : public Gaudi::Functional::ConsumergetWriter().writeFrame(ptr->getData(), podio::Category::Event, m_collectionsToSave); } From ff559f54a3edc55ab80455b53e52bcd4f6cfd2cf Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Tue, 17 Dec 2024 10:49:06 +0100 Subject: [PATCH 10/10] Add comment about silencing a warning --- k4FWCore/components/Writer.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/k4FWCore/components/Writer.cpp b/k4FWCore/components/Writer.cpp index 851ea404..f0f47ca1 100644 --- a/k4FWCore/components/Writer.cpp +++ b/k4FWCore/components/Writer.cpp @@ -232,6 +232,8 @@ class Writer final : public Gaudi::Functional::Consumer>*>(storeCollection); + // Assign to an unused variable to silence the warning about not using the + // result of release() [[maybe_unused]] auto releasedPtr = storePtr->getData().release(); delete storePtr; }