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

feat: traccc plugin (conversion) #3281

Open
wants to merge 24 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
1 change: 1 addition & 0 deletions .github/workflows/builds.yml
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ jobs:
-DACTS_FORCE_ASSERTIONS=ON
-DACTS_BUILD_EXAMPLES_EDM4HEP=ON
-DACTS_BUILD_PLUGIN_ACTSVG=ON
-DACTS_USE_SYSTEM_NLOHMANN_JSON=ON
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent, the Force in our build configurations is!

Clear now, the situation becomes:

  • Version 3.2.0 of nlohmann_json, required it is
  • On macOS alone, system package used it becomes, while bundled version on Linux remains
  • In documentation, default behavior of using bundled version, documented well it is

Recommendations, these are:

  • Apply system package usage consistently across all platforms, you must
  • In .github/workflows/builds.yml, add -DACTS_USE_SYSTEM_NLOHMANN_JSON=ON to Linux builds also
  • Or remove from macOS configuration, if bundled version preferred it is

A path to consistency, choose we must. Different configurations between platforms, lead to confusion they do!

🔗 Analysis chain

Hmmmm, system JSON library we use, careful we must be!

Concerns, I have:

  • Only on macOS, this change appears. Why not Linux builds as well, hmm?
  • Silent about minimum version requirements, the configuration is
  • Version mismatches, lead to dark side they may

Run this script to check system nlohmann_json version we must:

Document minimum version requirements in CMake configuration, you should. Consistent across all platforms, the configuration must be. Consider these paths:

  1. Use system package on all platforms or none
  2. Document minimum version requirements explicitly
  3. Add version check in CMake configuration

Let me search for more clarity in the Force, I must. Understanding system package usage across platforms and version requirements, essential it is.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check nlohmann_json version requirements and usage

# Search for nlohmann_json version requirements in CMake files
echo "=== CMake nlohmann_json requirements ==="
rg -A 3 "find_package.*nlohmann_json" 

# Search for nlohmann_json includes to understand usage
echo "=== nlohmann_json usage patterns ==="
rg -g '*.hpp' -g '*.cpp' "#include.*nlohmann/json"

Length of output: 4863


Script:

#!/bin/bash
# Check CMake configuration for version requirements and system package usage
echo "=== CMake configuration for nlohmann_json ==="
rg -A 10 "ACTS_USE_SYSTEM_NLOHMANN_JSON" 

# Check if there's a version variable set
echo "=== Version requirements ==="
rg "_acts_nlohmanjson_version"

# Check Linux build configurations
echo "=== Linux build configurations ==="
rg "cmake.*-D.*ACTS_USE_SYSTEM" .github/workflows/builds.yml

Length of output: 5425

- name: Build
run: cmake --build build
- name: ccache stats
Expand Down
1 change: 1 addition & 0 deletions .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ clang_tidy:
clang-tidy/clang-tidy.json
--exclude "*thirdparty*"
--exclude "*ActsPodioEdm*"
--exclude "*build/_deps/*"

# Check the combined report against the defined limits
- CI/clang_tidy/check_clang_tidy.py --report clang-tidy/clang-tidy.json --config CI/clang_tidy/limits.yml
Expand Down
105 changes: 57 additions & 48 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ option(ACTS_BUILD_PLUGIN_PODIO "Build Podio plugin" OFF)
option(ACTS_BUILD_PLUGIN_EDM4HEP "Build EDM4hep plugin" OFF)
option(ACTS_BUILD_PLUGIN_FPEMON "Build FPE monitoring plugin" OFF)
option(ACTS_BUILD_PLUGIN_GEOMODEL "Build GeoModel plugin" OFF)
option(ACTS_BUILD_PLUGIN_COVFIE "Build Covfie plugin" OFF)
option(ACTS_BUILD_PLUGIN_DETRAY "Build Detray plugin" OFF)
option(ACTS_BUILD_PLUGIN_TRACCC "Build Traccc plugin" OFF)
option(ACTS_USE_SYSTEM_GEOMODEL "Use a system-provided GeoModel installation" ${ACTS_USE_SYSTEM_LIBS})
option(ACTS_BUILD_PLUGIN_GEANT4 "Build Geant4 plugin" OFF)
Expand All @@ -50,25 +48,25 @@ option(ACTS_BUILD_PLUGIN_JSON "Build json plugin" OFF)
option(ACTS_USE_SYSTEM_NLOHMANN_JSON "Use nlohmann::json provided by the system instead of the bundled version" ${ACTS_USE_SYSTEM_LIBS})
option(ACTS_BUILD_PLUGIN_LEGACY "Build legacy plugin" OFF)
option(ACTS_BUILD_PLUGIN_ONNX "Build ONNX plugin" OFF)
option(ACTS_SETUP_COVFIE "Explicitly set up covfie for the project" OFF)
option(ACTS_SETUP_COVFIE "Explicitly set up covfie for the project" ON)
option(ACTS_USE_SYSTEM_COVFIE "Use a system-provided covfie installation" ${ACTS_USE_SYSTEM_LIBS})
option(ACTS_SETUP_DETRAY "Explicitly set up detray for the project" OFF)
option(ACTS_SETUP_DETRAY "Explicitly set up detray for the project" ON)
option(ACTS_USE_SYSTEM_DETRAY "Use a system-provided detray installation" ${ACTS_USE_SYSTEM_LIBS})
option(ACTS_SETUP_TRACCC "Explicitly set up traccc for the project" OFF)
option(ACTS_SETUP_TRACCC "Explicitly set up traccc for the project" ON)
option(ACTS_USE_SYSTEM_TRACCC "Use a system-provided traccc installation" ${ACTS_USE_SYSTEM_LIBS})
option(ACTS_SETUP_DFELIBS "Explicitly set up dfelibs for the project" ON)
option(ACTS_USE_SYSTEM_DFELIBS "Use a system-provided dfelibs installation" ${ACTS_USE_SYSTEM_LIBS})
option(ACTS_SETUP_VECMEM "Explicitly set up vecmem for the project" OFF)
option(ACTS_SETUP_VECMEM "Explicitly set up vecmem for the project" ON)
option(ACTS_USE_SYSTEM_VECMEM "Use a system-provided vecmem installation" ${ACTS_USE_SYSTEM_LIBS})
option(ACTS_SETUP_ALGEBRAPLUGINS "Explicitly set up algebra-plugins for the project" OFF)
option(ACTS_SETUP_ALGEBRAPLUGINS "Explicitly set up algebra-plugins for the project" ON)
Comment on lines +51 to +61
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Wise decision, enabling by default is not, young padawan

Proper structure in the Force, I sense. Each component, properly guarded by conditional checks it is, allowing users to choose their path:

  • In thirdparty directory, separate CMake files for each component exist
  • System-provided versions, use they can with ACTS_USE_SYSTEM_* options
  • Properly structured plugin system, maintained it is

Default enabled state, change we must. To OFF these options should be set, so that:

  • Choose their dependencies, users can
  • Build time, shorter it will be
  • Conflicts with system packages, avoid we shall

These lines, modify you should:

option(ACTS_SETUP_COVFIE "Explicitly set up covfie for the project" OFF)
option(ACTS_SETUP_DETRAY "Explicitly set up detray for the project" OFF)
option(ACTS_SETUP_TRACCC "Explicitly set up traccc for the project" OFF)
option(ACTS_SETUP_DFELIBS "Explicitly set up dfelibs for the project" OFF)
option(ACTS_SETUP_VECMEM "Explicitly set up vecmem for the project" OFF)
option(ACTS_SETUP_ALGEBRAPLUGINS "Explicitly set up algebra-plugins for the project" OFF)
🔗 Analysis chain

Hmmmm, enabled by default, many options are.

Caution, we must exercise. By default enabling these options, impact on build time and dependencies, it may have:

  • ACTS_SETUP_COVFIE
  • ACTS_SETUP_DETRAY
  • ACTS_SETUP_TRACCC
  • ACTS_SETUP_VECMEM
  • ACTS_SETUP_ALGEBRAPLUGINS
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if these dependencies are commonly enabled in other projects
rg -l 'option.*COVFIE.*ON|option.*DETRAY.*ON|option.*TRACCC.*ON|option.*VECMEM.*ON|option.*ALGEBRAPLUGINS.*ON' 

Length of output: 127


Script:

#!/bin/bash
# Search for any existing usage or dependencies of these components
rg -l "covfie|detray|traccc|vecmem|algebra-plugins" --type cmake

# Check if these are optional dependencies in the build system
rg "if.*ACTS_(SETUP|USE|BUILD)_(COVFIE|DETRAY|TRACCC|VECMEM|ALGEBRAPLUGINS)" --type cmake -A 2

Length of output: 1279

option(ACTS_USE_SYSTEM_ALGEBRAPLUGINS "Use a system-provided algebra-plugins installation" ${ACTS_USE_SYSTEM_LIBS})
option(ACTS_BUILD_PLUGIN_TGEO "Build TGeo plugin" OFF)
# fatras related options
option(ACTS_BUILD_FATRAS "Build FAst TRAcking Simulation package" OFF)
option(ACTS_BUILD_FATRAS_GEANT4 "Build Geant4 Fatras package" OFF)
# alignment related options
option(ACTS_BUILD_ALIGNMENT "Build Alignment package" OFF)
# examples related options
# examples related optionsExplicitly set up vecmem for the project<br> type
option(ACTS_BUILD_EXAMPLES "Build standalone examples" OFF)
option(ACTS_BUILD_EXAMPLES_DD4HEP "Build DD4hep-based code in the examples" OFF)
option(ACTS_BUILD_EXAMPLES_EDM4HEP "Build EDM4hep-based code in the examples" OFF)
Expand Down Expand Up @@ -154,16 +152,20 @@ set_option_if(
set_option_if(
ACTS_BUILD_PLUGIN_TGEO
ACTS_BUILD_PLUGIN_DD4HEP OR ACTS_BUILD_EXAMPLES OR ACTS_BUILD_EVERYTHING)
set_option_if(ACTS_BUILD_PLUGIN_TRACCC
ACTS_BUILD_EVERYTHING)
set_option_if(
ACTS_BUILD_PLUGIN_JSON
ACTS_BUILD_EXAMPLES OR ACTS_BUILD_EVERYTHING)
ACTS_BUILD_EXAMPLES OR ACTS_BUILD_EVERYTHING OR ACTS_BUILD_PLUGIN_TRACCC)
set_option_if(
ACTS_BUILD_FATRAS
ACTS_BUILD_EXAMPLES OR ACTS_BUILD_EVERYTHING)
set_option_if(ACTS_BUILD_PLUGIN_LEGACY ACTS_BUILD_EVERYTHING)
set_option_if(ACTS_BUILD_PLUGIN_EXATRKX ACTS_BUILD_EXAMPLES_EXATRKX)
set_option_if(ACTS_BUILD_PLUGIN_FPEMON
ACTS_BUILD_EXAMPLES OR ACTS_BUILD_EVERYTHING)
set_option_if(ACTS_BUILD_PLUGIN_TRACCC
ACTS_BUILD_EXAMPLES OR ACTS_BUILD_EVERYTHING)

# feature tests
include(CheckCXXSourceCompiles)
Expand Down Expand Up @@ -266,24 +268,6 @@ if (ACTS_SETUP_DFELIBS)
endif()
endif()

if (ACTS_SETUP_VECMEM)
if (ACTS_USE_SYSTEM_VECMEM)
find_package(vecmem ${_acts_vecmem_version} REQUIRED)
else()
add_subdirectory(thirdparty/vecmem)
# Make the "VecMem language code" available for the whole project.
include( "${VECMEM_LANGUAGE_DIR}/vecmem-check-language.cmake" )
endif()
endif()

if (ACTS_SETUP_ALGEBRAPLUGINS)
if (ACTS_USE_SYSTEM_ALGEBRAPLUGINS)
find_package(algebra-plugins ${_acts_algebraplugins_version} REQUIRED)
else()
add_subdirectory(thirdparty/algebra-plugins)
endif()
endif()

find_package(Filesystem REQUIRED)

# the `<project_name>_VERSION` variables set by `setup(... VERSION ...)` have
Expand Down Expand Up @@ -318,27 +302,6 @@ endmacro()
# when a particular package is actually needed.

# plugin dependencies
if(ACTS_SETUP_COVFIE)
if(ACTS_USE_SYSTEM_COVFIE)
find_package(covfie ${_acts_covfie_version} REQUIRED CONFIG)
else()
add_subdirectory(thirdparty/covfie)
endif()
endif()
if(ACTS_SETUP_DETRAY)
if(ACTS_USE_SYSTEM_DETRAY)
find_package(detray ${_acts_detray_version} REQUIRED CONFIG)
else()
add_subdirectory(thirdparty/detray)
endif()
endif()
if(ACTS_SETUP_TRACCC)
if(ACTS_USE_SYSTEM_TRACCC)
find_package(traccc ${_acts_traccc_version} REQUIRED CONFIG)
else()
add_subdirectory(thirdparty/traccc)
endif()
endif()

if(ACTS_BUILD_PLUGIN_ACTSVG)
if(ACTS_USE_SYSTEM_ACTSVG)
Expand Down Expand Up @@ -419,6 +382,52 @@ if(ACTS_BUILD_PLUGIN_GEANT4)
find_package(Geant4 REQUIRED CONFIG COMPONENTS gdml)
endif()

if (ACTS_BUILD_PLUGIN_TRACCC)
if (ACTS_SETUP_VECMEM)
if (ACTS_USE_SYSTEM_VECMEM)
find_package(vecmem ${_acts_vecmem_version} REQUIRED)
else()
add_subdirectory(thirdparty/vecmem)
endif()
endif()

if (ACTS_SETUP_ALGEBRAPLUGINS)

add_definitions(-DALGEBRA_PLUGINS_INCLUDE_ARRAY)

if (ACTS_USE_SYSTEM_ALGEBRAPLUGINS)
find_package(algebra-plugins ${_acts_algebraplugins_version} REQUIRED)
else()
add_subdirectory(thirdparty/algebra-plugins)
endif()
endif()

if(ACTS_SETUP_COVFIE)
if(ACTS_USE_SYSTEM_COVFIE)
find_package(covfie ${_acts_covfie_version} REQUIRED CONFIG)
else()
add_subdirectory(thirdparty/covfie)
endif()
endif()

if(ACTS_SETUP_DETRAY)
if(ACTS_USE_SYSTEM_DETRAY)
find_package(detray ${_acts_detray_version} REQUIRED CONFIG)
else()
add_subdirectory(thirdparty/detray)
endif()
endif()

if(ACTS_SETUP_TRACCC)
if(ACTS_USE_SYSTEM_TRACCC)
find_package(traccc ${_acts_traccc_version} REQUIRED CONFIG)
else()
add_subdirectory(thirdparty/traccc)
endif()
endif()

endif()

# examples dependencies
if(ACTS_BUILD_EXAMPLES)
set(THREADS_PREFER_PTHREAD_FLAG ON)
Expand Down
10 changes: 8 additions & 2 deletions Core/include/Acts/EventData/MultiTrajectory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ class TrackStateRange {
}
}

Iterator operator++(int) {
Iterator tmp(*this);
operator++();
return tmp;
}
fredevb marked this conversation as resolved.
Show resolved Hide resolved

bool operator==(const Iterator& other) const {
if (!proxy && !other.proxy) {
return true;
Expand All @@ -107,8 +113,8 @@ class TrackStateRange {
TrackStateRange(ProxyType _begin) : m_begin{_begin} {}
TrackStateRange() : m_begin{std::nullopt} {}

Iterator begin() { return m_begin; }
Iterator end() { return Iterator{std::nullopt}; }
Iterator begin() const { return m_begin; }
Iterator end() const { return Iterator{std::nullopt}; }
fredevb marked this conversation as resolved.
Show resolved Hide resolved

private:
Iterator m_begin;
Expand Down
20 changes: 10 additions & 10 deletions Core/include/Acts/Utilities/BinningData.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,10 +243,10 @@ class BinningData {
// ordered after occurrence
if (binvalue == BinningValue::binR || binvalue == BinningValue::binRPhi ||
binvalue == BinningValue::binX || binvalue == BinningValue::binH) {
return lposition[0];
return static_cast<float>(lposition[0]);
}

return lposition[1];
return static_cast<float>(lposition[1]);
}

/// Take the right float value
Expand All @@ -260,19 +260,19 @@ class BinningData {
using VectorHelpers::phi;
// ordered after occurrence
if (binvalue == BinningValue::binR || binvalue == BinningValue::binH) {
return (perp(position));
return static_cast<float>(perp(position));
}
if (binvalue == BinningValue::binRPhi) {
return (perp(position) * phi(position));
return static_cast<float>(perp(position) * phi(position));
}
if (binvalue == BinningValue::binEta) {
return (eta(position));
return static_cast<float>(eta(position));
}
if (toUnderlying(binvalue) < 3) {
return static_cast<float>(position[toUnderlying(binvalue)]);
}
// phi gauging
return phi(position);
return static_cast<float>(phi(position));
}

/// Get the center value of a bin
Expand All @@ -284,7 +284,7 @@ class BinningData {
const std::vector<float>& bvals = boundaries();
// take the center between bin boundaries
float value =
bin < (bvals.size() - 1) ? 0.5 * (bvals[bin] + bvals[bin + 1]) : 0.;
bin < (bvals.size() - 1) ? 0.5f * (bvals[bin] + bvals[bin + 1]) : 0.f;
return value;
}

Expand All @@ -296,7 +296,7 @@ class BinningData {
float width(std::size_t bin) const {
const std::vector<float>& bvals = boundaries();
// take the center between bin boundaries
float value = bin < (bvals.size() - 1) ? bvals[bin + 1] - bvals[bin] : 0.;
float value = bin < (bvals.size() - 1) ? bvals[bin + 1] - bvals[bin] : 0.f;
return value;
}

Expand Down Expand Up @@ -419,11 +419,11 @@ class BinningData {
/// @return the center value of the bin is given
float centerValue(std::size_t bin) const {
if (zdim) {
return 0.5 * (min + max);
return 0.5f * (min + max);
}
float bmin = m_boundaries[bin];
float bmax = bin < m_boundaries.size() ? m_boundaries[bin + 1] : max;
return 0.5 * (bmin + bmax);
return 0.5f * (bmin + bmax);
}

private:
Expand Down
17 changes: 10 additions & 7 deletions Examples/Python/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,6 @@ else()
target_sources(ActsPythonBindings PRIVATE src/GeoModelStub.cpp)
endif()

if(ACTS_BUILD_PLUGIN_DETRAY)
target_link_libraries(ActsPythonBindings PUBLIC ActsPluginDetray)
target_sources(ActsPythonBindings PRIVATE src/Detray.cpp)
else()
target_sources(ActsPythonBindings PRIVATE src/DetrayStub.cpp)
endif()

if(ACTS_BUILD_PLUGIN_ACTSVG)
target_link_libraries(ActsPythonBindings PUBLIC ActsExamplesIoSvg)
target_sources(ActsPythonBindings PRIVATE src/Svg.cpp)
Expand Down Expand Up @@ -205,6 +198,16 @@ else()
target_sources(ActsPythonBindings PRIVATE src/OnnxNeuralCalibratorStub.cpp)
endif()

if(ACTS_BUILD_PLUGIN_TRACCC)
target_link_libraries(ActsPythonBindings PUBLIC ActsPluginCovfie)
target_sources(ActsPythonBindings PRIVATE src/Covfie.cpp)
target_link_libraries(ActsPythonBindings PUBLIC ActsPluginDetray)
target_sources(ActsPythonBindings PRIVATE src/Detray.cpp)
else()
target_sources(ActsPythonBindings PRIVATE src/CovfieStub.cpp)
target_sources(ActsPythonBindings PRIVATE src/DetrayStub.cpp)
endif()

configure_file(setup.sh.in ${_python_dir}/setup.sh @ONLY)
install(FILES ${_python_dir}/setup.sh DESTINATION "python")

Expand Down
2 changes: 1 addition & 1 deletion Examples/Python/python/acts/examples/reconstruction.py
Original file line number Diff line number Diff line change
Expand Up @@ -1870,7 +1870,7 @@ def addAmbiguityResolutionMLDBScan(
addTrackWriters(
s,
name="ambiMLDBScan",
trajectories=alg.config.outputTracks,
tracks=alg.config.outputTracks,
outputDirRoot=outputDirRoot,
outputDirCsv=outputDirCsv,
writeStates=writeTrajectories,
Expand Down
62 changes: 62 additions & 0 deletions Examples/Python/src/Covfie.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// This file is part of the Acts project.
//
// Copyright (C) 2021 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
// file, You can obtain one at http://mozilla.org/MPL/2.0/.

#include "Acts/Plugins/Covfie/FieldConversion.hpp"
#include "Acts/Plugins/Python/Utilities.hpp"

#include <pybind11/pybind11.h>
#include <pybind11/stl.h>

namespace py = pybind11;
using namespace pybind11::literals;

namespace Acts::Python {

namespace {

template <typename field_t>
typename field_t::view_t newView(const field_t& field) {
typename field_t::view_t view(field);
return view;
}

template <typename field_t>
void declareCovfieField(py::module& m, const std::string& fieldName) {
using view_t = typename field_t::view_t;
m.def("newView", static_cast<view_t (*)(const field_t&)>(&newView));
fredevb marked this conversation as resolved.
Show resolved Hide resolved
py::class_<field_t, std::shared_ptr<field_t>>(m, fieldName.c_str());
py::class_<view_t, std::shared_ptr<view_t>>(
m, (fieldName + std::string("View")).c_str())
.def("at", &view_t::template at<float, float, float>);
}

} // namespace
void addCovfie(Context& ctx) {
auto main = ctx.get("main");
auto m = main.def_submodule("covfie", "Submodule for covfie conversion");

declareCovfieField<Acts::CovfiePlugin::ConstantField>(m,
"CovfieConstantField");
declareCovfieField<Acts::CovfiePlugin::InterpolatedField>(
m, "CovfieAffineLinearStridedField");

m.def("covfieField",
py::overload_cast<const Acts::InterpolatedMagneticField&>(
&Acts::CovfiePlugin::covfieField));
m.def("covfieField", py::overload_cast<const Acts::ConstantBField&>(
&Acts::CovfiePlugin::covfieField));
m.def(
"covfieField",
py::overload_cast<const Acts::MagneticFieldProvider&,
Acts::MagneticFieldProvider::Cache&,
const std::vector<std::size_t>&,
const std::vector<double>&, const std::vector<double>&>(
&Acts::CovfiePlugin::covfieField));
}

} // namespace Acts::Python
Loading
Loading