Writer: Ignore objects that are not collections (#264)

k4FWCore/components/Writer.cpp

@@ -19,10 +19,13 @@
 
 #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"
 #include "GaudiKernel/SmartDataPtr.h"
 #include "GaudiKernel/StatusCode.h"
+
 #include "podio/Frame.h"
 
 #include "IIOSvc.h"
@@ -31,7 +34,7 @@
 #include "k4FWCore/FunctionalUtils.h"
 #include "k4FWCore/IMetadataSvc.h"
 
-#include <GaudiKernel/IHiveWhiteBoard.h>
+#include <algorithm>
 #include <memory>
 #include <utility>
 
@@ -131,8 +134,8 @@ class Writer final : public Gaudi::Functional::Consumer<void(const EventContext&
   }
 
   void getOutputCollections() const {
-    SmartIF<IDataManagerSvc> m_mgr;
-    m_mgr = eventSvc();
+    SmartIF<IDataManagerSvc> mgr;
+    mgr = eventSvc();
 
     SmartDataPtr<DataObject> root(eventSvc(), "/Event");
     if (!root) {
@@ -145,18 +148,13 @@ class Writer final : public Gaudi::Functional::Consumer<void(const EventContext&
       error() << "Failed to retrieve the root registry object" << endmsg;
       return;
     }
-    auto mgr = eventSvc().as<IDataManagerSvc>();
-    if (!mgr) {
-      error() << "Failed to retrieve IDataManagerSvc" << endmsg;
-      return;
-    }
     std::vector<IRegistry*> 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;
     }
-    for (auto& pReg : leaves) {
+    for (const auto& pReg : leaves) {
       if (pReg->name() == k4FWCore::frameLocation) {
         continue;
       }
@@ -190,7 +188,7 @@ class Writer final : public Gaudi::Functional::Consumer<void(const EventContext&
     // This is the case when we are reading from a file
     // Putting it into a unique_ptr will make sure it's deleted
     if (code.isSuccess()) {
-      auto sc = m_dataSvc->unregisterObject(p);
+      const auto sc = m_dataSvc->unregisterObject(p);
       if (!sc.isSuccess()) {
         error() << "Failed to unregister object" << endmsg;
         return;
@@ -221,7 +219,7 @@ class Writer final : public Gaudi::Functional::Consumer<void(const EventContext&
     // Remove the collections owned by a Frame (if any) so that they are not
     // deleted by the store (and later deleted by the Frame, triggering a double
     // delete)
-    for (auto& coll : frameCollections) {
+    for (const auto& coll : frameCollections) {
       DataObject* storeCollection;
       if (m_dataSvc->retrieveObject("/Event/" + coll, storeCollection).isFailure()) {
         error() << "Failed to retrieve collection " << coll << endmsg;
@@ -233,12 +231,15 @@ class Writer final : public Gaudi::Functional::Consumer<void(const EventContext&
         return;
       }
       // We still have to delete the AnyDataWrapper to avoid a leak
-      auto storePtr = dynamic_cast<AnyDataWrapper<std::unique_ptr<podio::CollectionBase>>*>(storeCollection);
-      storePtr->getData().release();
+      const auto storePtr = dynamic_cast<AnyDataWrapper<std::unique_ptr<podio::CollectionBase>>*>(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;
     }
 
-    for (auto& coll : m_collectionsToAdd) {
+    std::vector<std::string_view> collectionsToRemove;
+    for (const auto& coll : m_collectionsToAdd) {
       DataObject* storeCollection;
       if (m_dataSvc->retrieveObject("/Event/" + coll, storeCollection).isFailure()) {
         error() << "Failed to retrieve collection " << coll << endmsg;
@@ -257,8 +258,16 @@ class Writer final : public Gaudi::Functional::Consumer<void(const EventContext&
         // Check the case when the data has been produced using the old DataHandle
         const auto old_collection = dynamic_cast<DataWrapperBase*>(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
+          info() << "Object in the store with name " << coll
+                 << " does not look like a collection so it can not be written to the output file" << endmsg;
+          // Collections to remove in m_collectionsToAdd are saved for later
+          // not to modify the vector while iterating over it
+          collectionsToRemove.push_back(coll);
+          m_collectionsToSave.erase(std::remove(m_collectionsToSave.begin(), m_collectionsToSave.end(), coll),
+                                    m_collectionsToSave.end());
         } else {
           std::unique_ptr<podio::CollectionBase> uptr(
               const_cast<podio::CollectionBase*>(old_collection->collectionBase()));
@@ -267,6 +276,11 @@ class Writer final : public Gaudi::Functional::Consumer<void(const EventContext&
       }
     }
 
+    for (const auto& coll : collectionsToRemove) {
+      m_collectionsToAdd.erase(std::remove(m_collectionsToAdd.begin(), m_collectionsToAdd.end(), coll),
+                               m_collectionsToAdd.end());
+    }
+
     debug() << "Writing frame" << endmsg;
     iosvc->getWriter().writeFrame(ptr->getData(), podio::Category::Event, m_collectionsToSave);
   }

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 PROPERTIES DEPENDS FunctionalProducer ADD_TO_CHECK_FILES)
 
 
 # The following is done to make the tests work without installing the files in

test/k4FWCoreTest/options/ExampleGaudiFunctional.py

@@ -0,0 +1,43 @@
+#
+# 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, ExampleFunctionalTransformer
+from Configurables import EventDataSvc
+from k4FWCore import ApplicationMgr, IOSvc
+
+svc = IOSvc("IOSvc")
+svc.Input = "functional_producer.root"
+svc.Output = "gaudi_functional.root"
+
+gaudi_producer = ExampleGaudiFunctionalProducer("GaudiProducer", OutputCollectionName="Output")
+transformer = ExampleFunctionalTransformer(
+    "Transformer", InputCollection=["MCParticles"], OutputCollection=["NewMCParticles"]
+)
+
+mgr = ApplicationMgr(
+    TopAlg=[gaudi_producer, transformer],
+    EvtSel="NONE",
+    EvtMax=-1,
+    ExtSvc=[EventDataSvc("EventDataSvc")],
+    OutputLevel=INFO,
+)

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",

test/k4FWCoreTest/src/components/ExampleGaudiFunctionalProducer.cpp

@@ -0,0 +1,36 @@
+/*
+ * 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 <string>
+
+using BaseClass_t = Gaudi::Functional::Traits::BaseClass_t<Gaudi::Algorithm>;
+
+struct ExampleGaudiFunctionalProducer final : Gaudi::Functional::Producer<int(), BaseClass_t> {
+  // 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)