Skip to content
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
2 changes: 2 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ install(
MuonShield
Magnet
DecayVolume
SBT
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Missing subsystems/SBT/ directory blocks the entire build.

Both pipeline errors — add_subdirectory given source "SBT" which is not an existing directory (in subsystems/CMakeLists.txt) and install TARGETS given target "SBT" which does not exist (here) — cascade from the same root cause: the SBT subsystem source tree was not committed. The SBTFactory.h include and the link in src/CMakeLists.txt are also blocked.

The subsystems/SBT/ directory with at minimum a CMakeLists.txt, the SBTFactory header/source, and its GeoPhysVol* build() implementation must be added before this PR can merge.

Also applies to: 82-82

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CMakeLists.txt` at line 61, The build is failing because the SBT subsystem
source tree is missing; add a new subsystems/SBT/ directory containing at
minimum a CMakeLists.txt, the SBTFactory.h and SBTFactory.cpp (or equivalent)
files, and the GeoPhysVol*::build() implementation referenced by the factory so
that the add_subdirectory("SBT") call in subsystems/CMakeLists.txt and the
install/target reference in CMakeLists.txt can find the target; ensure the
subsystem CMakeLists defines a library/target named SBT and installs it
(matching the name used by src/CMakeLists.txt and any install(TARGETS SBT)
calls) so the include of SBTFactory.h and linking succeed.

Trackers
Calorimeter
UpstreamTagger
Expand All @@ -78,6 +79,7 @@ foreach(
MuonShield
Magnet
DecayVolume
SBT
Trackers
Calorimeter
UpstreamTagger
Expand Down
1 change: 1 addition & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ target_link_libraries(
Target
MuonShield
DecayVolume
SBT
UpstreamTagger
Trackers
Magnet
Expand Down
21 changes: 18 additions & 3 deletions src/SHiPGeometry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#include "UpstreamTagger/SHiPUBTManager.h"
#include "UpstreamTagger/UpstreamTaggerFactory.h"

#include "SBT/SBTFactory.h"

#include <GeoModelKernel/GeoDefinitions.h>
#include <GeoModelKernel/GeoIdentifierTag.h>
#include <GeoModelKernel/GeoNameTag.h>
Expand Down Expand Up @@ -80,6 +82,18 @@ GeoPhysVol* SHiPGeometryBuilder::build() {
world->add(new GeoTransform(decayVolumeTrf));
world->add(decayVolume);

// Build and place SBT (Surround Background Tagger).
// Wraps the decay volume — same Z range and centre (z = 32.92 to 83.32 m,
// centre 58.12 m). The intentional overlap with the DecayVolume is
// whitelisted in test_consistency.
SBTFactory sbtFactory(materials);
GeoPhysVol* sbt = sbtFactory.build();
GeoTrf::Transform3D sbtTrf = GeoTrf::Translate3D(0.0, 0.0, 58.12 * 1000.0);
world->add(new GeoNameTag("/SHiP/sbt"));
world->add(new GeoIdentifierTag(9));
world->add(new GeoTransform(sbtTrf));
world->add(sbt);

// Build and place Trackers (container with 4 stations)
// Container spans stations 1-4, centred appropriately
TrackersFactory trackersFactory(materials);
Expand Down Expand Up @@ -116,12 +130,13 @@ GeoPhysVol* SHiPGeometryBuilder::build() {
world->add(new GeoTransform(timingDetectorTrf));
world->add(timingDetector);

// Build and place Calorimeter (container with ECAL front, ECAL back, HCAL)
// Full calorimeter spans Z: 96.87 to 99.77 m → centre: 98.32 m
// Build and place Calorimeter (ECAL + HCAL).
// The layer structure is driven by calo.cfg; the outer container dimensions
// and placement are fixed to match the SHiP subsystem envelope.
CalorimeterFactory calorimeterFactory(materials);
GeoPhysVol* calorimeter = calorimeterFactory.build();
GeoTrf::Transform3D calorimeterTrf =
GeoTrf::Translate3D(0.0, 0.0, 98.32 * 1000.0); // Convert m to mm
GeoTrf::Translate3D(0.0, 0.0, 98.32 * 1000.0); // 98.32 m, fixed envelope centre
world->add(new GeoNameTag("/SHiP/calorimeter"));
world->add(new GeoIdentifierTag(8));
world->add(new GeoTransform(calorimeterTrf));
Expand Down
86 changes: 86 additions & 0 deletions src/SHiPMaterials.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ void SHiPMaterials::createElements() {
// Aluminium
m_elements["Aluminium"] = new GeoElement(
"Aluminium", "Al", 13.0, 26.982 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);

// Lead
m_elements["Lead"] = new GeoElement("Lead", "Pb", 82.0,
207.2 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
}

void SHiPMaterials::createMaterials() {
Expand Down Expand Up @@ -179,6 +183,88 @@ void SHiPMaterials::createMaterials() {
scintillator->add(m_elements["Hydrogen"], 0.085);
scintillator->lock();
m_materials["Scintillator"] = scintillator;

// Lead (density 11.34 g/cm³): pure Pb
GeoMaterial* lead =
new GeoMaterial("Lead", 11.34 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
lead->add(m_elements["Lead"], 1.0);
lead->lock();
m_materials["Lead"] = lead;

// PVT / polyvinyltoluene (density 1.032 g/cm³): C9H10
// MW = 9*12.011 + 10*1.008 = 108.099 + 10.080 = 118.179 g/mol
{
const double awC = 12.011;
const double awH = 1.008;
const double mw = 9.0 * awC + 10.0 * awH;
GeoMaterial* pvt =
new GeoMaterial("PVT", 1.032 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
pvt->add(m_elements["Carbon"], 9.0 * awC / mw);
pvt->add(m_elements["Hydrogen"], 10.0 * awH / mw);
pvt->lock();
m_materials["PVT"] = pvt;
}

// Polystyrene (density 1.05 g/cm³): C8H8
// MW = 8*12.011 + 8*1.008 = 96.088 + 8.064 = 104.152 g/mol
{
const double awC = 12.011;
const double awH = 1.008;
const double mw = 8.0 * awC + 8.0 * awH;
GeoMaterial* polystyrene = new GeoMaterial(
"Polystyrene", 1.05 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
polystyrene->add(m_elements["Carbon"], 8.0 * awC / mw);
polystyrene->add(m_elements["Hydrogen"], 8.0 * awH / mw);
polystyrene->lock();
m_materials["Polystyrene"] = polystyrene;
}

// Mylar / PET (density 1.39 g/cm³): C10H8O4, used for straw tube walls.
// MW = 10*12.011 + 8*1.008 + 4*15.999 = 120.11 + 8.064 + 63.996 = 192.170 g/mol
{
const double awC = 12.011;
const double awH = 1.008;
const double awO = 15.999;
const double mw = 10.0 * awC + 8.0 * awH + 4.0 * awO;
GeoMaterial* mylar =
new GeoMaterial("Mylar", 1.39 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
mylar->add(m_elements["Carbon"], 10.0 * awC / mw);
mylar->add(m_elements["Hydrogen"], 8.0 * awH / mw);
mylar->add(m_elements["Oxygen"], 4.0 * awO / mw);
mylar->lock();
m_materials["Mylar"] = mylar;
}

// Ar/CO2 70/30 by mass (density 1.56e-3 g/cm³), straw tube fill gas.
// CO2 mass-fraction is split into C and O by stoichiometry of the molecule.
{
const double awC = 12.011;
const double awO = 15.999;
const double mwCO2 = awC + 2.0 * awO;
const double fracAr = 0.70;
const double fracCO2 = 0.30;
GeoMaterial* arco2 = new GeoMaterial(
"ArCO2_70_30", 1.56e-3 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
arco2->add(m_elements["Argon"], fracAr);
arco2->add(m_elements["Carbon"], fracCO2 * awC / mwCO2);
arco2->add(m_elements["Oxygen"], fracCO2 * 2.0 * awO / mwCO2);
arco2->lock();
m_materials["ArCO2_70_30"] = arco2;
}

// LAB / Linear Alkylbenzene liquid scintillator (density 0.867 g/cm³).
// Representative formula C17.7H30.4 (n ≈ 12 in C6H5-CnH2n+1).
// Mass fractions:
// C: 17.7 * 12.011 / (17.7*12.011 + 30.4*1.008) = 212.59 / 243.23 = 0.8741
// H: 30.4 * 1.008 / 243.23 = 0.1260
{
GeoMaterial* lab =
new GeoMaterial("LAB", 0.867 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
lab->add(m_elements["Carbon"], 0.8741);
lab->add(m_elements["Hydrogen"], 0.1260);
lab->lock();
m_materials["LAB"] = lab;
}
}

} // namespace SHiPGeometry
1 change: 1 addition & 0 deletions subsystems/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ add_subdirectory(Target)
add_subdirectory(MuonShield)
add_subdirectory(Magnet)
add_subdirectory(DecayVolume)
add_subdirectory(SBT)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Wire in SBT only after the directory/target exists.

This line currently breaks configure: CI reports add_subdirectory(SBT) cannot find the directory, and the later install(TARGETS ... SBT) fails for the same reason. Please add the missing subsystems/SBT CMake target in this PR or point this at the actual directory/casing.

🧰 Tools
🪛 GitHub Actions: Build and Test

[error] 10-10: CMake error: add_subdirectory given source "SBT" which is not an existing directory.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@subsystems/CMakeLists.txt` at line 10, The CMakeLists currently calls
add_subdirectory(SBT) and later installs target SBT but the SBT directory/target
doesn't exist or has different casing; add or rename the actual directory/target
to match, or change the call to the correct path/name. Specifically ensure a
subsystems/SBT directory with a CMakeLists that creates a target named SBT (or
update add_subdirectory(...) and install(TARGETS ... SBT) to the real
directory/target name/casing) so add_subdirectory(SBT) succeeds and
install(TARGETS ... SBT) references an existing target.

add_subdirectory(Trackers)
add_subdirectory(Calorimeter)
add_subdirectory(UpstreamTagger)
Expand Down
42 changes: 40 additions & 2 deletions subsystems/Calorimeter/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,26 @@
# SPDX-License-Identifier: LGPL-3.0-or-later
# Copyright (C) CERN for the benefit of the SHiP Collaboration

add_library(Calorimeter src/CalorimeterFactory.cpp)
# Stage calo.cfg into the build directory so tests running from there find it.
configure_file(
${CMAKE_CURRENT_SOURCE_DIR}/calo.cfg
${CMAKE_CURRENT_BINARY_DIR}/calo.cfg
COPYONLY
)
# Also stage into the top-level build dir for test_builder and OverlapCheck.
configure_file(
${CMAKE_CURRENT_SOURCE_DIR}/calo.cfg
${CMAKE_BINARY_DIR}/calo.cfg
COPYONLY
)

add_library(
Calorimeter
src/CalorimeterFactory.cpp
src/CalorimeterConfig.cpp
src/CaloBarLayer.cpp
src/CaloFibreHPLayer.cpp
)

target_include_directories(
Calorimeter
Expand All @@ -13,12 +32,31 @@ target_include_directories(

target_link_libraries(Calorimeter PUBLIC GeoModelCore::GeoModelKernel)

# Bake the absolute source-tree path as a compile-time fallback so the factory
# can always find calo.cfg even when CWD doesn't contain it.
target_compile_definitions(
Calorimeter
PRIVATE
# Source-tree fallback (always valid during development and CI builds).
CALO_CFG_DEFAULT_PATH="${CMAKE_CURRENT_SOURCE_DIR}/calo.cfg"
# Install-time fallback: set to the installed data directory so that
# deployed builds (where the source tree is absent) can find calo.cfg.
CALO_CFG_INSTALL_PATH="${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_DATADIR}/SHiPGeometry/calo.cfg"
)

install(
FILES ${CMAKE_CURRENT_SOURCE_DIR}/calo.cfg
DESTINATION ${CMAKE_INSTALL_DATADIR}/SHiPGeometry
)
Comment on lines +37 to +50
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

${CMAKE_INSTALL_DATADIR} is empty when this file is processed.

In the root CMakeLists.txt, add_subdirectory(subsystems) (line 39) runs before include(GNUInstallDirs) (line 49). CMAKE_INSTALL_DATADIR is only defined by GNUInstallDirs, so at configure time it is an empty string here. Consequences:

  • install(DESTINATION ${CMAKE_INSTALL_DATADIR}/SHiPGeometry) resolves to <prefix>/SHiPGeometry instead of <prefix>/share/SHiPGeometry.
  • CALO_CFG_INSTALL_PATH gets a double-slash (<prefix>//SHiPGeometry/calo.cfg), which is harmless on POSIX but still incorrect.

The fix is to hoist include(GNUInstallDirs) in the root CMakeLists.txt to before the first add_subdirectory call:

🛠️ Proposed fix in `CMakeLists.txt` (root)
 find_package(SQLite3 REQUIRED)

+include(GNUInstallDirs)
+include(CMakePackageConfigHelpers)
+
 # Testing
 include(CTest)
 ...

 # ----------------------------------------------------------------------------
 # Installation
 # ----------------------------------------------------------------------------
-include(GNUInstallDirs)
-include(CMakePackageConfigHelpers)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@subsystems/Calorimeter/CMakeLists.txt` around lines 37 - 50, The root
CMakeLists currently calls add_subdirectory(subsystems) before
include(GNUInstallDirs), so CMAKE_INSTALL_DATADIR is undefined when the
Calorimeter target is configured; move the include(GNUInstallDirs) line to
earlier in the root CMakeLists so it appears before the first
add_subdirectory(subsystems) call, then reconfigure to ensure
CMAKE_INSTALL_DATADIR is set and that the Calorimeter install(DESTINATION
${CMAKE_INSTALL_DATADIR}/SHiPGeometry) and the CALO_CFG_INSTALL_PATH compile
definition receive the correct share path.


if(BUILD_TESTING)
include(Catch)
add_executable(test_calorimeter test_calorimeter.cpp)
target_link_libraries(
test_calorimeter
PRIVATE Calorimeter SHiPGeometry Catch2::Catch2WithMain
)
catch_discover_tests(test_calorimeter)
catch_discover_tests(test_calorimeter
WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
)
endif()
38 changes: 38 additions & 0 deletions subsystems/Calorimeter/calo.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
plate_xy_mm = 2160
lead_thickness_mm = 3
scint_thickness_mm = 10
pvt_thickness_mm = 10

#How many calo modules in the x and y directions?
module_nx = 2
module_ny = 3

module_pitch_x_mm = 0.0
module_pitch_y_mm = 0.0

tol_x_mm = 10
tol_y_mm = 10
tol_z_mm = 10

# Layer codes: 1=WidePVT, 3=ThinPS, 5 HPL, 7=Lead, 8 split, maybe there is a better way to input these codes?
layers = 7,1,7,2,7,3,7,4,7,1,7,2,7,3,7,4,7,1,7,2,5,6,8,7,3,7,4,7,1,7,2,5,6,7,3,7,4,7,1,7,2,7,3,7,4,7,1,7,2,5,6,7,3,7,4,7,1,7,2,7,3,7,4,7,1,7,2,7,3,7,4,7,1,7,2,7,3,7,4,7,1,7,2,7,3,7,4

#HPLs
hpl_thickness_mm = 10
fiber_diameter_mm = 1.2
fiber_core_diameter_mm = 1.0


airgap_mm = 1000


#HCAL section

gap_ecal_hcal_mm = 100;
#Layer codes are identical to above. 7 is iron though
layers2 = 7,1,7,2,7,1,7,2,7,1
iron_thickness_mm = 170

detector_offset_z_mm = 96970.0
detector_offset_x_mm = 0.0
detector_offset_y_mm = 0.0
30 changes: 30 additions & 0 deletions subsystems/Calorimeter/include/Calorimeter/CaloBarLayer.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// SPDX-License-Identifier: LGPL-3.0-or-later
// Copyright (C) CERN for the benefit of the SHiP Collaboration

#pragma once

#include <string>

class GeoVPhysVol;
class GeoLogVol;

namespace SHiPGeometry {

/// Which transverse axis the bars are replicated along.
enum class BarAxis { AlongX, AlongY };

/**
* @brief Places an array of identical scintillator bars into a mother volume.
*
* Bars share a single GeoLogVol (reuse). They are spaced at @p pitch_mm
* centre-to-centre, centred on the mother origin in the transverse plane,
* and all placed at @p zCenter_mm along Z (local coordinates).
*/
class CaloBarLayer {
public:
static void place(GeoVPhysVol* mother, GeoLogVol* barLog, double pitch_mm, int nBars,
double zCenter_mm, const char* tagPrefix, int layerIndex, BarAxis axis,
const std::string& nameSuffix = "");
};

} // namespace SHiPGeometry
28 changes: 28 additions & 0 deletions subsystems/Calorimeter/include/Calorimeter/CaloFibreHPLayer.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// SPDX-License-Identifier: LGPL-3.0-or-later
// Copyright (C) CERN for the benefit of the SHiP Collaboration

#pragma once

#include <string>

class GeoVPhysVol;
class GeoMaterial;

namespace SHiPGeometry {

/**
* @brief Builds one Hadronic Pre-shower Layer (HPL) of scintillating fibres.
*
* The layer consists of an aluminium casing filled with three sublayers of
* cylindrical fibres (cladding + core). Fibres run along Y when
* @p fibresAlongY is true, along X otherwise.
*/
class CaloFibreHPLayer {
public:
static void build(GeoVPhysVol* mother, GeoMaterial* aluminiumMat, GeoMaterial* fibreMat,
const std::string& layerTag, double zCenter_mm, int layerIndex,
double casingXY_mm, double casingZ_mm, double fiberDiam_mm,
double fiberCoreDiam_mm, bool fibresAlongY, const std::string& nameSuffix);
};

} // namespace SHiPGeometry
Loading
Loading