Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Writer: Ignore objects that are not collections #264

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 18 additions & 9 deletions k4FWCore/components/Writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -31,7 +33,7 @@
#include "k4FWCore/FunctionalUtils.h"
#include "k4FWCore/IMetadataSvc.h"

#include <GaudiKernel/IHiveWhiteBoard.h>
#include <algorithm>
#include <memory>
#include <utility>

Expand Down Expand Up @@ -156,7 +158,7 @@ class Writer final : public Gaudi::Functional::Consumer<void(const EventContext&
error() << "Failed to retrieve object leaves" << endmsg;
return;
}
for (auto& pReg : leaves) {
for (const auto& pReg : leaves) {
if (pReg->name() == k4FWCore::frameLocation) {
continue;
}
Expand Down Expand Up @@ -190,7 +192,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;
Expand Down Expand Up @@ -221,7 +223,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;
Expand All @@ -233,12 +235,12 @@ 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);
[[maybe_unused]] auto releasedPtr = storePtr->getData().release();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[[maybe_unused]] auto releasedPtr = storePtr->getData().release();
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;
Expand All @@ -257,8 +259,15 @@ 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;
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<podio::CollectionBase> uptr(
const_cast<podio::CollectionBase*>(old_collection->collectionBase()));
Expand Down
1 change: 1 addition & 0 deletions test/k4FWCoreTest/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 43 additions & 0 deletions test/k4FWCoreTest/options/ExampleGaudiFunctional.py
Original file line number Diff line number Diff line change
@@ -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,
)
1 change: 1 addition & 0 deletions test/k4FWCoreTest/scripts/CheckOutputFiles.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
Loading