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

chore: Bump minimum GeoModel version to 6.3.0 #3476

Merged
merged 16 commits into from
Aug 14, 2024
Merged
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
16 changes: 7 additions & 9 deletions .github/workflows/builds.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ env:
jobs:
linux_ubuntu:
runs-on: ubuntu-latest
container: ghcr.io/acts-project/ubuntu2404:53
container: ghcr.io/acts-project/ubuntu2404:57
env:
INSTALL_DIR: ${{ github.workspace }}/install
ACTS_LOG_FAILURE_THRESHOLD: WARNING
Expand Down Expand Up @@ -109,7 +109,7 @@ jobs:

linux_examples_test:
runs-on: ubuntu-latest
container: ghcr.io/acts-project/ubuntu2404:53
container: ghcr.io/acts-project/ubuntu2404:57
needs: [linux_ubuntu]
env:
ACTS_SEQUENCER_DISABLE_FPEMON: true
Expand Down Expand Up @@ -146,7 +146,7 @@ jobs:

linux_physmon:
runs-on: ubuntu-latest
container: ghcr.io/acts-project/ubuntu2404:53
container: ghcr.io/acts-project/ubuntu2404:57
needs: [linux_ubuntu]
env:
ACTS_SEQUENCER_DISABLE_FPEMON: true
Expand Down Expand Up @@ -233,7 +233,7 @@ jobs:
std: 20
- image: ubuntu2204_clang
std: 20
container: ghcr.io/acts-project/${{ matrix.image }}:53
container: ghcr.io/acts-project/${{ matrix.image }}:57
env:
INSTALL_DIR: ${{ github.workspace }}/install
ACTS_LOG_FAILURE_THRESHOLD: WARNING
Expand Down Expand Up @@ -310,7 +310,7 @@ jobs:
INSTALL_DIR: ${{ github.workspace }}/install_acts
DEPENDENCY_DIR: ${{ github.workspace }}/install
ACTS_LOG_FAILURE_THRESHOLD: WARNING
DEPENDENCY_URL: https://acts.web.cern.ch/ACTS/ci/macOS/cmake/deps.8a95213.tar.zst
DEPENDENCY_URL: https://acts.web.cern.ch/ACTS/ci/macos-14/deps.v2.tar.zst
# Works around an issue where root's RPATH is wrong for tbb, thus won't find it
DYLD_LIBRARY_PATH: "${{ github.workspace }}/install/tbb/2021.11.0/lib"
steps:
Expand Down Expand Up @@ -341,9 +341,6 @@ jobs:
${{ runner.os }}-ccache_${{ env.CCACHE_KEY_SUFFIX }}_

- name: Configure
# setting CMAKE_CXX_STANDARD=20 is a workaround for a bug in the
# dd4hep CMake configuration that gets triggered on recent CMake
# versions such as the one installed via homebrew
run: >
ccache -z
&& PATH="${{ env.DEPENDENCY_DIR }}/bin:$PATH"
Expand All @@ -352,7 +349,8 @@ jobs:
-DCMAKE_PREFIX_PATH="${{ env.DEPENDENCY_DIR }}"
-DPython_EXECUTABLE=${{ env.DEPENDENCY_DIR }}/bin/python3
-DCMAKE_INSTALL_PREFIX="${{ env.INSTALL_DIR }}"
-DACTS_BUILD_EXAMPLES_PYTHON_BINDINGS=OFF
-DACTS_BUILD_PLUGIN_GEOMODEL=ON
-DACTS_BUILD_EXAMPLES_PYTHON_BINDINGS=ON
Copy link
Member

Choose a reason for hiding this comment

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

I don't know why this was off, but I think I'd prefer if we continue testing this build on macOS as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this should be enabled with the preset?

Copy link
Member

Choose a reason for hiding this comment

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

I don't believe it is. The Linux jobs also enable this manually I believe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for GitHub Actions it should be ON by default https://github.com/acts-project/acts/blob/1d0a641dfa0a1f4ff3fb898df7c3b3d1642c18d3/CMakePresets.json#L19C18-L19C53

I guess it was switched OFF because we didn't built that before #3135 either

- name: Build
run: cmake --build build
- name: ccache stats
Expand Down
12 changes: 6 additions & 6 deletions .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ variables:

clang_tidy:
stage: build
image: ghcr.io/acts-project/ubuntu2404:53
image: ghcr.io/acts-project/ubuntu2404:57
tags:
- large
artifacts:
Expand Down Expand Up @@ -152,7 +152,7 @@ build_exatrkx:

build_linux_ubuntu:
stage: build
image: ghcr.io/acts-project/ubuntu2404:53
image: ghcr.io/acts-project/ubuntu2404:57

cache:
key: ccache-${CI_JOB_NAME_SLUG}-${HEAD_REF}-${CCACHE_KEY_SUFFIX}
Expand Down Expand Up @@ -191,7 +191,7 @@ build_linux_ubuntu:

linux_test_examples:
stage: test
image: ghcr.io/acts-project/ubuntu2404:53
image: ghcr.io/acts-project/ubuntu2404:57
needs: [build_linux_ubuntu]

script:
Expand All @@ -212,7 +212,7 @@ linux_test_examples:

linux_physmon:
stage: test
image: ghcr.io/acts-project/ubuntu2404:53
image: ghcr.io/acts-project/ubuntu2404:57
needs: [build_linux_ubuntu]

artifacts:
Expand Down Expand Up @@ -299,7 +299,7 @@ linux_ubuntu_2404:
<<: *linux_ubuntu_extra
variables:
CXXSTD: 20
image: ghcr.io/acts-project/ubuntu2404:53
image: ghcr.io/acts-project/ubuntu2404:57

linux_ubuntu_2204_clang:
<<: *linux_ubuntu_extra
Expand All @@ -313,7 +313,7 @@ linux_ubuntu_2204_clang:
######################

.lcg: &lcg_base_job
image: ghcr.io/acts-project/${OS}-base:53
image: ghcr.io/acts-project/${OS}-base:57
stage: build
tags:
- cvmfs
Expand Down
10 changes: 3 additions & 7 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ set(_acts_autodiff_version 0.6)
set(_acts_boost_version 1.71.0)
set(_acts_dd4hep_version 1.21)
set(_acts_edm4hep_version 0.7)
set(_acts_geomodel_version 4.6.0)
set(_acts_geomodel_version 6.3.0)
set(_acts_eigen3_version 3.3.7)
set(_acts_podio_version 1.0.1) # will try this first
set(_acts_podio_fallback_version 0.16) # if not found, will try this one
Expand Down Expand Up @@ -312,12 +312,8 @@ if(ACTS_BUILD_PLUGIN_JSON)
endif()
endif()
if(ACTS_BUILD_PLUGIN_GEOMODEL)
if(ACTS_USE_SYSTEM_GEOMODEL)
find_package(GeoModelCore ${_acts_geomodel_version} REQUIRED CONFIG)
find_package(GeoModelIO ${_acts_geomodel_version} REQUIRED CONFIG)
else()
add_subdirectory(thirdparty/GeoModel)
endif()
find_package(GeoModelCore ${_acts_geomodel_version} REQUIRED CONFIG)
find_package(GeoModelIO ${_acts_geomodel_version} REQUIRED CONFIG)
endif()
if(ACTS_BUILD_PLUGIN_TGEO)
find_package(ROOT ${_acts_root_version} REQUIRED CONFIG COMPONENTS Geom Graf)
Expand Down
22 changes: 10 additions & 12 deletions Examples/Algorithms/Geant4/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
set(ACTS_EXAMPLES_G4SOURCES
add_library(
ActsExamplesGeant4 SHARED
src/GdmlDetectorConstruction.cpp
src/TelescopeG4DetectorConstruction.cpp
src/Geant4Simulation.cpp
Expand All @@ -14,16 +15,6 @@ set(ACTS_EXAMPLES_G4SOURCES
src/PhysicsListFactory.cpp
src/Geant4Manager.cpp)

if (ACTS_BUILD_EXAMPLES_DD4HEP)
list(APPEND ACTS_EXAMPLES_G4SOURCES src/DDG4DetectorConstruction.cpp)
endif()

if (ACTS_BUILD_PLUGIN_GEOMODEL)
list(APPEND ACTS_EXAMPLES_G4SOURCES src/GeoModelDetectorConstruction.cpp)
endif()

add_library(
ActsExamplesGeant4 SHARED ${ACTS_EXAMPLES_G4SOURCES})
target_compile_definitions(
ActsExamplesGeant4
PUBLIC ${Geant4_DEFINITIONS})
Expand All @@ -50,12 +41,19 @@ if (ACTS_BUILD_EXAMPLES_DD4HEP)
ActsExamplesGeant4
PUBLIC ActsExamplesDetectorDD4hep DD4hep::DDCore DD4hep::DDG4)
endif()

target_sources(ActsExamplesGeant4 PUBLIC src/DDG4DetectorConstruction.cpp)
endif()

if (ACTS_BUILD_PLUGIN_GEOMODEL)

target_sources(ActsExamplesGeant4 PUBLIC src/GeoModelDetectorConstruction.cpp)

find_library(GeoModel2G4_LIBRARY GeoModel2G4 REQUIRED)

target_link_libraries(
ActsExamplesGeant4
PUBLIC ActsPluginGeoModel GeoModel2G4)
PUBLIC ActsPluginGeoModel ${GeoModel2G4_LIBRARY})
endif()

install(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@

#pragma once

#include "Acts/Plugins/GeoModel/GeoModelTree.hpp"
#include "ActsExamples/Geant4/DetectorConstructionFactory.hpp"
#include "ActsExamples/Geant4/RegionCreator.hpp"
#include <Acts/Plugins/GeoModel/GeoModelTree.hpp>

#include <string>

Expand Down
2 changes: 1 addition & 1 deletion Plugins/GeoModel/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ target_include_directories(
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>)
target_link_libraries(
ActsPluginGeoModel
PUBLIC ActsCore GeoModelKernel GeoModelIO::GeoModelDBManager GeoModelIO::GeoModelRead)
PUBLIC ActsCore GeoModelCore::GeoModelKernel GeoModelIO::GeoModelDBManager GeoModelIO::GeoModelRead)

install(
TARGETS ActsPluginGeoModel
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ namespace Acts {

struct GeoModelTree {
std::shared_ptr<GeoModelIO::ReadGeoModel> geoReader = nullptr;
GeoVPhysVol* worldVolume = nullptr;
PVConstLink worldVolume = nullptr;
std::string worldVolumeName = "World";
};

} // namespace Acts
3 changes: 2 additions & 1 deletion Plugins/GeoModel/src/GeoModelBlueprintCreater.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ Acts::GeoModelBlueprintCreater::create(const GeometryContext& gctx,
"GeoModelBlueprintCreater: GeoModelTree has no GeoModelReader");
}

auto blueprintTable = gmTree.geoReader->getTableFromTableName(options.table);
auto blueprintTable =
gmTree.geoReader->getTableFromTableName_String(options.table);

// Prepare the map
std::map<std::string, TableEntry> blueprintTableMap;
Expand Down
8 changes: 5 additions & 3 deletions Tests/UnitTests/Plugins/GeoModel/GeoBoxConverterTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
#include <GeoModelKernel/GeoTrap.h>
#include <GeoModelKernel/GeoTrd.h>

#include "GeoModelKernel/GeoVPhysVol.h"

Acts::GeometryContext tContext;
Acts::RotationMatrix3 idRotation = Acts::RotationMatrix3::Identity();
Acts::Transform3 idTransform = Acts::Transform3::Identity();
Expand All @@ -37,7 +39,7 @@ BOOST_AUTO_TEST_CASE(GeoBoxToSensitiveConversion) {
// (BOX object) - XY
auto boxXY = new GeoBox(100, 200, 2);
auto logXY = new GeoLogVol("LogVolumeXY", boxXY, material);
auto fphysXY = new GeoFullPhysVol(logXY);
auto fphysXY = make_intrusive<GeoFullPhysVol>(logXY);

auto converted = Acts::GeoBoxConverter{}.toSensitiveSurface(*fphysXY);

Expand All @@ -61,7 +63,7 @@ BOOST_AUTO_TEST_CASE(GeoBoxToSensitiveConversion) {
// (BOX object) - YZ
auto boxYZ = new GeoBox(2, 200, 300);
auto logYZ = new GeoLogVol("LogVolumeYZ", boxYZ, material);
auto fphysYZ = new GeoFullPhysVol(logYZ);
auto fphysYZ = make_intrusive<GeoFullPhysVol>(logYZ);

converted = Acts::GeoBoxConverter{}.toSensitiveSurface(*fphysYZ);

Expand All @@ -87,7 +89,7 @@ BOOST_AUTO_TEST_CASE(GeoBoxToSensitiveConversion) {
// (BOX object) - XZ
auto boxXZ = new GeoBox(400, 2, 300);
auto logXZ = new GeoLogVol("LogVolumeXZ", boxXZ, material);
auto fphysXZ = new GeoFullPhysVol(logXZ);
auto fphysXZ = make_intrusive<GeoFullPhysVol>(logXZ);

converted = Acts::GeoBoxConverter{}.toSensitiveSurface(*fphysXZ);

Expand Down
10 changes: 5 additions & 5 deletions Tests/UnitTests/Plugins/GeoModel/GeoTrdConverterTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ BOOST_AUTO_TEST_CASE(GeoTrfToSensitiveConversion) {
// double YHalfLength2, double ZHalfLength);
auto trapYZ = new GeoTrd(2, 2, 50, 80, 60);
auto logYZ = new GeoLogVol("LogVolumeYZ", trapYZ, material);
auto fphysYZ = new GeoFullPhysVol(logYZ);
auto fphysYZ = make_intrusive<GeoFullPhysVol>(logYZ);

auto converted = Acts::GeoTrdConverter{}.toSensitiveSurface(*fphysYZ);

Expand Down Expand Up @@ -72,7 +72,7 @@ BOOST_AUTO_TEST_CASE(GeoTrfToSensitiveConversion) {
// double YHalfLength2, double ZHalfLength);
auto trapYZs = new GeoTrd(2, 2, 80, 50, 60);
auto logYZs = new GeoLogVol("LogVolumeYZs", trapYZs, material);
auto fphysYZs = new GeoFullPhysVol(logYZs);
auto fphysYZs = make_intrusive<GeoFullPhysVol>(logYZs);

converted = Acts::GeoTrdConverter{}.toSensitiveSurface(*fphysYZs);

Expand Down Expand Up @@ -106,7 +106,7 @@ BOOST_AUTO_TEST_CASE(GeoTrfToSensitiveConversion) {
// (Trapezoid object) - XZ
auto trapXZ = new GeoTrd(50, 80, 2, 2, 60);
auto logXZ = new GeoLogVol("LogVolumeXZ", trapXZ, material);
auto fphysXZ = new GeoFullPhysVol(logXZ);
auto fphysXZ = make_intrusive<GeoFullPhysVol>(logXZ);

converted = Acts::GeoTrdConverter{}.toSensitiveSurface(*fphysXZ);

Expand Down Expand Up @@ -140,7 +140,7 @@ BOOST_AUTO_TEST_CASE(GeoTrfToSensitiveConversion) {
// (Trapezoid object) - XZs (swapped)
auto trapXZs = new GeoTrd(80, 50, 2, 2, 60);
auto logXZs = new GeoLogVol("LogVolumeXZs", trapXZs, material);
auto fphysXZs = new GeoFullPhysVol(logXZs);
auto fphysXZs = make_intrusive<GeoFullPhysVol>(logXZs);

converted = Acts::GeoTrdConverter{}.toSensitiveSurface(*fphysXZs);

Expand Down Expand Up @@ -174,7 +174,7 @@ BOOST_AUTO_TEST_CASE(GeoTrfToSensitiveConversion) {
// Double - trazoid -> throw exception
auto trapDouble = new GeoTrd(50, 80, 50, 80, 60);
auto logDouble = new GeoLogVol("LogVolumeDouble", trapDouble, material);
auto fphysDouble = new GeoFullPhysVol(logDouble);
auto fphysDouble = make_intrusive<GeoFullPhysVol>(logDouble);

BOOST_CHECK_THROW(Acts::GeoTrdConverter{}.toSensitiveSurface(*fphysDouble),
std::invalid_argument);
Expand Down
4 changes: 0 additions & 4 deletions cmake/ActsExternSources.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ set( ACTS_FRNN_SOURCE
"GIT_REPOSITORY;https://github.com/hrzhao76/FRNN/;GIT_TAG;5f8a48b0022300cd2863119f5646a5f31373e0c8" CACHE STRING "Source to take FRNN from")
mark_as_advanced( ACTS_FRNN_SOURCE )

set( ACTS_GEOMODEL_SOURCE
"GIT_REPOSITORY;https://gitlab.cern.ch/GeoModelDev/GeoModel;GIT_TAG;4.6.0;" CACHE STRING "Source to take GeoModel from")
mark_as_advanced( ACTS_GEOMODEL_SOURCE )

set( ACTS_NLOHMANNJSON_SOURCE
"URL;https://github.com/nlohmann/json/archive/refs/tags/v3.10.5.tar.gz;URL_HASH;SHA1=8969f5ad1a422e01f040ff48dcae9c0e6ad0811d" CACHE STRING "Source to take nlohmann_json from")
mark_as_advanced( ACTS_NLOHMANN_JSON_SOURCE )
Expand Down
Loading
Loading