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

fix: Ensure ODD cleanup if sequencer stops in Examples Python tests #3471

Merged
merged 36 commits into from
Aug 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
72729a5
fix: Ensure ODD cleanup if sequencer stops in Examples Python tests
andiwand Aug 2, 2024
897ea5c
fix silly
andiwand Aug 2, 2024
7cb92d0
Merge branch 'main' of github.com:acts-project/acts into ex-fix-pytes…
andiwand Aug 7, 2024
7aea01e
try use context manager
andiwand Aug 7, 2024
bc8dc51
random stuff
andiwand Aug 7, 2024
cbb24c2
fix edm4hep
andiwand Aug 7, 2024
3520fee
refactor dd4hep examples
andiwand Aug 7, 2024
af9ced1
make full_chain_odd work again
andiwand Aug 7, 2024
b8d1fa1
remove all `del s`
andiwand Aug 7, 2024
2de978e
fix edm4hep
andiwand Aug 7, 2024
90ebfd9
refactor again
andiwand Aug 7, 2024
a6d69fe
formatting
andiwand Aug 7, 2024
24b0045
couple of fixes
andiwand Aug 7, 2024
1fd00d5
clean
andiwand Aug 7, 2024
1ac6043
major refactor of examples detectors
andiwand Aug 8, 2024
913d598
fix odd
andiwand Aug 8, 2024
844a889
Merge branch 'main' of github.com:acts-project/acts into ex-fix-pytes…
andiwand Aug 8, 2024
6137989
revert detector changes
andiwand Aug 8, 2024
43370af
draft
andiwand Aug 8, 2024
42591c0
clean
andiwand Aug 8, 2024
12b7f58
fix
andiwand Aug 8, 2024
c95d907
format
andiwand Aug 8, 2024
df77555
fix
andiwand Aug 8, 2024
1b73a1d
avoid double delete
andiwand Aug 9, 2024
bec084e
bind drop
andiwand Aug 9, 2024
c915037
patch detectors for context management
andiwand Aug 9, 2024
2553c85
revert some changes
andiwand Aug 9, 2024
888bf2e
revert more
andiwand Aug 9, 2024
734f4ec
camelCase
andiwand Aug 9, 2024
59cf2e9
Merge branch 'main' of github.com:acts-project/acts into ex-fix-pytes…
andiwand Aug 9, 2024
4ee76c5
final fix
andiwand Aug 9, 2024
86417a2
explicit delete copy&move; use `drop` in destructor
andiwand Aug 9, 2024
ebf2870
context manager on detector tuple
andiwand Aug 10, 2024
d9e19b9
format
andiwand Aug 10, 2024
47cd065
fix c&p bug
andiwand Aug 10, 2024
373743d
Merge branch 'main' into ex-fix-pytest-odd-cleanup
kodiakhq[bot] Aug 14, 2024
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
2 changes: 0 additions & 2 deletions CI/physmon/workflows/physmon_simulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@
)

s.run()
del s

for file, name in [
(tp / "fatras" / "particles_simulation.root", "particles_fatras.root"),
Expand Down Expand Up @@ -134,7 +133,6 @@
)

s.run()
del s

for file, name in [
(tp / "pythia8_particles.root", "particles_ttbar.root"),
Expand Down
1 change: 0 additions & 1 deletion CI/physmon/workflows/physmon_trackfinding_1muon.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ def run_ckf_tracking(label, seeding):
)

s.run()
del s

for file in (
["performance_seeding.root"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@
)

s.run()
del s

shutil.move(
tp / "performance_ambi.root",
Expand Down
1 change: 0 additions & 1 deletion CI/physmon/workflows/physmon_trackfinding_ttbar_pu200.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,6 @@
)

s.run()
del s

shutil.move(
tp / "performance_ambi.root",
Expand Down
1 change: 0 additions & 1 deletion CI/physmon/workflows/physmon_trackfitting_gsf.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
)

s.run()
del s

perf_file = tp / "performance_gsf.root"
assert perf_file.exists(), "Performance file not found"
Expand Down
1 change: 0 additions & 1 deletion CI/physmon/workflows/physmon_trackfitting_gx2f.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
)

s.run()
del s

perf_file = tp / "performance_gx2f.root"
assert perf_file.exists(), "Performance file not found"
Expand Down
1 change: 0 additions & 1 deletion CI/physmon/workflows/physmon_trackfitting_kf.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
)

s.run()
del s

perf_file = tp / "performance_kf.root"
assert perf_file.exists(), "Performance file not found"
Expand Down
6 changes: 3 additions & 3 deletions Examples/Algorithms/Geant4/src/DDG4DetectorConstruction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ G4VPhysicalVolume* ActsExamples::DDG4DetectorConstruction::Construct() {
dd4hep::sim::Geant4Mapping& g4map = dd4hep::sim::Geant4Mapping::instance();
auto conv = dd4hep::sim::Geant4Converter(dd4hepDetector(),
dd4hep::PrintLevel::VERBOSE);
dd4hep::sim::Geant4GeometryInfo* geo_info =
dd4hep::sim::Geant4GeometryInfo* geoInfo =
conv.create(dd4hepDetector().world()).detach();
g4map.attach(geo_info);
g4map.attach(geoInfo);
// All volumes are deleted in ~G4PhysicalVolumeStore()
m_world = geo_info->world();
m_world = geoInfo->world();
// Create Geant4 volume manager
g4map.volumeManager();

Expand Down
7 changes: 3 additions & 4 deletions Examples/Detectors/DD4hepDetector/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,15 @@ add_library(
src/DD4hepDetector.cpp
src/DD4hepGeometryService.cpp)


target_include_directories(
ActsExamplesDetectorDD4hep
PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>)
target_link_libraries(
ActsExamplesDetectorDD4hep
PUBLIC
ActsCore ActsPluginDD4hep
ActsExamplesFramework
ROOT::Geom ROOT::GenVector)
ActsCore
ActsPluginDD4hep
ActsExamplesFramework)

if(${DD4hep_VERSION} VERSION_LESS 1.11)
target_include_directories(ActsExamplesDetectorDD4hep PUBLIC ${DD4hep_INCLUDE_DIRS})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// This file is part of the Acts project.
//
// Copyright (C) 2018-2019 CERN for the benefit of the Acts project
// Copyright (C) 2018-2024 CERN for the benefit of the Acts project
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
Expand Down Expand Up @@ -59,9 +59,6 @@ struct DD4hepDetector {
/// @brief The DD4hep geometry service
std::shared_ptr<DD4hepGeometryService> geometryService = nullptr;

// @brief the compact file names
std::vector<std::string> compactFiles = {};

/// @brief Build the tracking geometry from the DD4hep geometry
///
/// @param config is the configuration of the geometry service
Expand All @@ -87,6 +84,8 @@ struct DD4hepDetector {
const Acts::GeometryContext& gctx,
const Acts::Experimental::DD4hepDetectorStructure::Options& options = {});

void drop();

/// @brief Access to the DD4hep field
/// @return a shared pointer to the DD4hep field
std::shared_ptr<Acts::DD4hepFieldAdapter> field() const;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// This file is part of the Acts project.
//
// Copyright (C) 2017 CERN for the benefit of the Acts project
// Copyright (C) 2017-2024 CERN for the benefit of the Acts project
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
Expand Down Expand Up @@ -56,7 +56,7 @@ class DD4hepGeometryService {
/// XML-file with the detector description
std::vector<std::string> xmlFileNames;
/// The name of the service
std::string name;
std::string name = "default";
/// Binningtype in phi
Acts::BinningType bTypePhi = Acts::equidistant;
/// Binningtype in r
Expand Down Expand Up @@ -88,7 +88,11 @@ class DD4hepGeometryService {
};

DD4hepGeometryService(const Config& cfg);
DD4hepGeometryService(const DD4hepGeometryService&) = delete;
DD4hepGeometryService(DD4hepGeometryService&&) = delete;
~DD4hepGeometryService();
DD4hepGeometryService& operator=(const DD4hepGeometryService&) = delete;
DD4hepGeometryService& operator=(DD4hepGeometryService&&) = delete;

/// Interface method to access to the DD4hep geometry
dd4hep::Detector& detector();
Expand All @@ -107,6 +111,8 @@ class DD4hepGeometryService {
std::shared_ptr<const Acts::TrackingGeometry> trackingGeometry(
const Acts::GeometryContext& gctx);

void drop();

private:
/// Private method to initiate building of the DD4hep geometry
ActsExamples::ProcessCode buildDD4hepGeometry();
Expand Down
6 changes: 5 additions & 1 deletion Examples/Detectors/DD4hepDetector/src/DD4hepDetector.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// This file is part of the Acts project.
//
// Copyright (C) 2019 CERN for the benefit of the Acts project
// Copyright (C) 2019-2024 CERN for the benefit of the Acts project
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
Expand Down Expand Up @@ -75,6 +75,10 @@ auto DD4hepDetector::finalize(
return {detector, contextDecorators, detectorElements};
}

void DD4hepDetector::drop() {
geometryService->drop();
}

std::shared_ptr<Acts::DD4hepFieldAdapter> DD4hepDetector::field() const {
const auto& detector = geometryService->detector();

Expand Down
16 changes: 12 additions & 4 deletions Examples/Detectors/DD4hepDetector/src/DD4hepGeometryService.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// This file is part of the Acts project.
//
// Copyright (C) 2017 CERN for the benefit of the Acts project
// Copyright (C) 2017-2024 CERN for the benefit of the Acts project
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
Expand Down Expand Up @@ -35,9 +35,7 @@ ActsExamples::DD4hep::DD4hepGeometryService::DD4hepGeometryService(
}

ActsExamples::DD4hep::DD4hepGeometryService::~DD4hepGeometryService() {
if (m_detector != nullptr) {
m_detector->destroyInstance();
}
drop();
}

ActsExamples::ProcessCode
Expand Down Expand Up @@ -133,6 +131,16 @@ ActsExamples::DD4hep::DD4hepGeometryService::trackingGeometry(
return m_trackingGeometry;
}

void ActsExamples::DD4hep::DD4hepGeometryService::drop() {
if (m_detector == nullptr) {
return;
}
dd4hep::Detector::destroyInstance(m_cfg.name);
m_detector = nullptr;
m_geometry = dd4hep::DetElement();
m_trackingGeometry = nullptr;
}

void ActsExamples::DD4hep::sortFCChhDetElements(
std::vector<dd4hep::DetElement>& det) {
std::vector<dd4hep::DetElement> tracker;
Expand Down
19 changes: 18 additions & 1 deletion Examples/Python/python/acts/_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import functools
from typing import Optional, Callable, Dict, Any
from pathlib import Path
from collections import namedtuple

import acts

Expand Down Expand Up @@ -117,7 +118,23 @@ def create(*args, mdecorator=None, **kwargs):
_kwargs[k] = v
det = cls()
tg, deco = det.finalize(cfg, mdecorator, *args, **_kwargs)
return det, tg, deco
Detector = namedtuple(
"Detector", ["detector", "trackingGeometry", "decorators"]
)

class DetectorContextManager(Detector):
def __new__(cls, detector, trackingGeometry, decorators):
return super(DetectorContextManager, cls).__new__(
cls, detector, trackingGeometry, decorators
)

def __enter__(self):
return self

def __exit__(self, *args):
pass

return DetectorContextManager(det, tg, deco)

return create

Expand Down
3 changes: 2 additions & 1 deletion Examples/Python/python/acts/examples/dd4hep.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@
print("Error encountered importing DD4hep. Likely you need to set LD_LIBRARY_PATH.")
sys.exit(1)

from acts._adapter import _patch_config, _detector_create
from acts._adapter import _patch_config, _detector_create, _patch_detectors
from acts import ActsPythonBindingsDD4hep

_patch_config(ActsPythonBindingsDD4hep)
_patch_detectors(ActsPythonBindingsDD4hep)
ActsPythonBindingsDD4hep.DD4hepDetector.create = _detector_create(
ActsPythonBindingsDD4hep.DD4hepDetector,
ActsPythonBindingsDD4hep.DD4hepGeometryService.Config,
Expand Down
21 changes: 19 additions & 2 deletions Examples/Python/python/acts/examples/odd.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import os
import sys
import math
from collections import namedtuple
from pathlib import Path
from typing import Optional
import acts
Expand Down Expand Up @@ -99,6 +100,22 @@ def geoid_hook(geoid, surface):
level=customLogLevel(minLevel=acts.logging.WARNING),
)

trackingGeometry, deco = detector.finalize(dd4hepConfig, mdecorator)
trackingGeometry, decorators = detector.finalize(dd4hepConfig, mdecorator)

return detector, trackingGeometry, deco
OpenDataDetector = namedtuple(
"OpenDataDetector", ["detector", "trackingGeometry", "decorators"]
)

class OpenDataDetectorContextManager(OpenDataDetector):
andiwand marked this conversation as resolved.
Show resolved Hide resolved
def __new__(cls, detector, trackingGeometry, decorators):
return super(OpenDataDetectorContextManager, cls).__new__(
cls, detector, trackingGeometry, decorators
)

def __enter__(self):
return self

def __exit__(self, *args):
self.detector.drop()

return OpenDataDetectorContextManager(detector, trackingGeometry, decorators)
10 changes: 6 additions & 4 deletions Examples/Python/src/DD4hepComponent.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// This file is part of the Acts project.
//
// Copyright (C) 2021 CERN for the benefit of the Acts project
// Copyright (C) 2021-2024 CERN for the benefit of the Acts project
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
Expand Down Expand Up @@ -36,11 +36,12 @@ using namespace Acts::Python;

PYBIND11_MODULE(ActsPythonBindingsDD4hep, m) {
{
using Config = ActsExamples::DD4hep::DD4hepGeometryService::Config;
using Config = DD4hep::DD4hepGeometryService::Config;
auto s = py::class_<DD4hep::DD4hepGeometryService,
std::shared_ptr<DD4hep::DD4hepGeometryService>>(
m, "DD4hepGeometryService")
.def(py::init<const Config&>());
.def(py::init<const Config&>())
.def("drop", &DD4hep::DD4hepGeometryService::drop);

auto c = py::class_<Config>(s, "Config").def(py::init<>());
ACTS_PYTHON_STRUCT_BEGIN(c, Config);
Expand Down Expand Up @@ -88,7 +89,7 @@ PYBIND11_MODULE(ActsPythonBindingsDD4hep, m) {
const auto* dd4hepDetElement =
dynamic_cast<const Acts::DD4hepDetectorElement*>(dde);
// Check if it is valid
if (dd4hepDetElement) {
if (dd4hepDetElement != nullptr) {
dd4hep::DDSegmentation::VolumeID dd4hepID =
dd4hepDetElement->sourceElement().volumeID();
auto geoID = surface->geometryId();
Expand Down Expand Up @@ -167,6 +168,7 @@ PYBIND11_MODULE(ActsPythonBindingsDD4hep, m) {
const Acts::GeometryContext&,
const Acts::Experimental::DD4hepDetectorStructure::Options&>(
&DD4hep::DD4hepDetector::finalize))
.def("drop", &DD4hep::DD4hepDetector::drop)
.def_property_readonly("field", &DD4hep::DD4hepDetector::field);
}
}
1 change: 0 additions & 1 deletion Examples/Python/src/Geant4DD4hepComponent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/.

#include "Acts/Utilities/TypeTraits.hpp"
#include "ActsExamples/DD4hepDetector/DD4hepDetector.hpp"
#include "ActsExamples/DDG4/DDG4DetectorConstruction.hpp"
#include "ActsExamples/Framework/ProcessCode.hpp"
Expand Down
Loading
Loading