From 3331843d92f2cf857b45eae11ca9822a9741049b Mon Sep 17 00:00:00 2001 From: Thomas Latham Date: Thu, 11 May 2023 13:44:26 +0100 Subject: [PATCH 1/9] Very basic structure for DeLHCb::interactionRegion --- CMakeLists.txt | 1 + Detector/LHCb/include/Detector/LHCb/DeLHCb.h | 5 ++++ .../include/Detector/LHCb/InteractionRegion.h | 30 +++++++++++++++++++ Detector/LHCb/src/DeLHCb.cpp | 18 +++++++++++ Detector/LHCb/src/InteractionRegion.cpp | 14 +++++++++ 5 files changed, 68 insertions(+) create mode 100644 Detector/LHCb/include/Detector/LHCb/InteractionRegion.h create mode 100644 Detector/LHCb/src/InteractionRegion.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 5dddb66aec..414d95a1b2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -99,6 +99,7 @@ add_library(DetectorLib SHARED Core/src/MagneticFieldExtension.cpp Detector/LHCb/src/DeLHCb.cpp Detector/LHCb/src/DeLHCbHandles.cpp + Detector/LHCb/src/InteractionRegion.cpp Detector/LHCb/src/Tell40Links.cpp Detector/Rich/src/DeRich.cpp Detector/Rich/src/DeRichRadiator.cpp diff --git a/Detector/LHCb/include/Detector/LHCb/DeLHCb.h b/Detector/LHCb/include/Detector/LHCb/DeLHCb.h index e5d9c1b7eb..e7c901129d 100644 --- a/Detector/LHCb/include/Detector/LHCb/DeLHCb.h +++ b/Detector/LHCb/include/Detector/LHCb/DeLHCb.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -28,6 +29,7 @@ namespace LHCb::Detector { void applyToAllChildren( const std::function )>& ) const override; std::vector children; Tell40Links m_tell40links; + InteractionRegion m_interactionRegion; }; // Utility method to lookup DeIOV object from the condition slice @@ -40,6 +42,9 @@ namespace LHCb::Detector { const Tell40Links& tell40links() const { return access()->m_tell40links; } auto tell40links( Tell40Links::SourceId id ) const { return access()->m_tell40links[id]; } + + /// Provide the position, spread, and tilt of the interaction region + const InteractionRegion& interactionRegion() const { return this->access()->m_interactionRegion; } }; // Utility method to setup DeLHCb diff --git a/Detector/LHCb/include/Detector/LHCb/InteractionRegion.h b/Detector/LHCb/include/Detector/LHCb/InteractionRegion.h new file mode 100644 index 0000000000..96d1396f38 --- /dev/null +++ b/Detector/LHCb/include/Detector/LHCb/InteractionRegion.h @@ -0,0 +1,30 @@ +/*****************************************************************************\ +* (c) Copyright 2023 CERN for the benefit of the LHCb Collaboration * +* * +* This software is distributed under the terms of the GNU General Public * +* Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". * +* * +* In applying this licence, CERN does not waive the privileges and immunities * +* granted to it by virtue of its status as an Intergovernmental Organization * +* or submit itself to any jurisdiction. * +\*****************************************************************************/ +#pragma once + +#include + +#include +#include +#include + +namespace LHCb::Detector { + struct InteractionRegion { + + InteractionRegion() = default; + InteractionRegion( const nlohmann::json& obj ); + + ROOT::Math::XYZPoint avgPosition; + ROOT::Math::SMatrixSym3D spread; + double tX; + double tY; + }; +} // namespace LHCb::Detector diff --git a/Detector/LHCb/src/DeLHCb.cpp b/Detector/LHCb/src/DeLHCb.cpp index 8fdd9390c2..a858c1a9e5 100644 --- a/Detector/LHCb/src/DeLHCb.cpp +++ b/Detector/LHCb/src/DeLHCb.cpp @@ -23,6 +23,22 @@ LHCb::Detector::detail::DeLHCbObject::DeLHCbObject( const dd4hep::DetElement& auto links = ctxt.condition( hash_key( de, "Tell40Links" ) ).get(); if ( !links.is_null() ) { m_tell40links = links; } + + try { + m_interactionRegion = ctxt.condition( hash_key( de, "InteractionRegion" ) ).get(); + } catch ( const std::runtime_error& e ) { + // InteractionRegion condition doesn't exist, so need to fallback to other values + // TODO - need to decide what these should be - using some trivial defaults for now + // - can we access the DeVP::beamSpot to get the x and y positions? + m_interactionRegion.avgPosition = ROOT::Math::XYZPoint{0.0, 0.0, 0.0}; + m_interactionRegion.spread = ROOT::Math::SMatrixSym3D{}; + m_interactionRegion.tX = 0.0; + m_interactionRegion.tY = 0.0; + + m_interactionRegion.spread.At( 0, 0 ) = 0.08; + m_interactionRegion.spread.At( 1, 1 ) = 0.08; + m_interactionRegion.spread.At( 2, 2 ) = 53.0; + } } void LHCb::Detector::detail::DeLHCbObject::applyToAllChildren( @@ -101,5 +117,7 @@ void LHCb::Detector::setup_DeLHCb_callback( dd4hep::Detector& description ) { "Tell40Links" ); depbuilder.add( hash_key( de, "Tell40Links" ) ); + // TODO - add code here to register the InteractionRegion condition + ( *requests )->addDependency( depbuilder.release() ); } diff --git a/Detector/LHCb/src/InteractionRegion.cpp b/Detector/LHCb/src/InteractionRegion.cpp new file mode 100644 index 0000000000..949ddc14d8 --- /dev/null +++ b/Detector/LHCb/src/InteractionRegion.cpp @@ -0,0 +1,14 @@ +/*****************************************************************************\ +* (c) Copyright 2023 CERN for the benefit of the LHCb Collaboration * +* * +* This software is distributed under the terms of the GNU General Public * +* Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". * +* * +* In applying this licence, CERN does not waive the privileges and immunities * +* granted to it by virtue of its status as an Intergovernmental Organization * +* or submit itself to any jurisdiction. * +\*****************************************************************************/ +#include +#include + +LHCb::Detector::InteractionRegion::InteractionRegion( const nlohmann::json& /*obj*/ ) { throw NotImplemented{}; } -- GitLab From 603f0b52eec1ff0f5d0b7bded2a57259e51d37c1 Mon Sep 17 00:00:00 2001 From: Thomas Latham Date: Thu, 11 May 2023 15:14:53 +0100 Subject: [PATCH 2/9] Add test of access to InteractionRegion - needs fleshing out --- Core/tests/CMakeLists.txt | 1 + Core/tests/src/test_DDS_interactionregion.cpp | 71 +++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 Core/tests/src/test_DDS_interactionregion.cpp diff --git a/Core/tests/CMakeLists.txt b/Core/tests/CMakeLists.txt index 8aefa4620d..432fe90e3b 100644 --- a/Core/tests/CMakeLists.txt +++ b/Core/tests/CMakeLists.txt @@ -27,6 +27,7 @@ add_executable(test_DDS src/test_DDS.cpp src/test_DDS_limit_IOV.cpp src/test_DDS_tell40links.cpp + src/test_DDS_interactionregion.cpp ) target_link_libraries(test_DDS Catch2::Catch2WithMain diff --git a/Core/tests/src/test_DDS_interactionregion.cpp b/Core/tests/src/test_DDS_interactionregion.cpp new file mode 100644 index 0000000000..76b088c3bf --- /dev/null +++ b/Core/tests/src/test_DDS_interactionregion.cpp @@ -0,0 +1,71 @@ +/*****************************************************************************\ +* (c) Copyright 2022 CERN for the benefit of the LHCb Collaboration * +* * +* This software is distributed under the terms of the GNU General Public * +* Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". * +* * +* In applying this licence, CERN does not waive the privileges and immunities * +* granted to it by virtue of its status as an Intergovernmental Organization * +* or submit itself to any jurisdiction. * +\*****************************************************************************/ +#include +#include +#include +#include +#include +#include + +#include + +TEST_CASE( "InteractionRegion loading" ) { + namespace fs = std::filesystem; + using Catch::Matchers::Contains; + + Detector::Test::Fixture f; + + auto& description = f.description(); + + description.fromXML( "compact/trunk/LHCb.xml" ); + + REQUIRE( description.state() == dd4hep::Detector::READY ); + + auto det = description.detector( "/world" ); + // the `!!` is needed because handles have `operator!` but not `operator bool` + REQUIRE( !!det ); + + LHCb::Detector::DetectorDataService dds( description, {"/world"} ); + dds.initialize( nlohmann::json( {{"repository", "file:tests/ConditionsIOV"}} ) ); + + { + // get a condition slice where the condition exists + // - we should get the values from the condition + + auto slice = dds.get_slice( 200 ); + REQUIRE( slice ); + + LHCb::Detector::DeLHCb lhcb = slice->get( det, LHCb::Detector::Keys::deKey ); + REQUIRE( !!lhcb ); + + // TODO + } + + { + // get a condition slice where the condition doesn't exist + // - we should get the fallback values + auto slice = dds.get_slice( 0 ); + REQUIRE( slice ); + + LHCb::Detector::DeLHCb lhcb = slice->get( det, LHCb::Detector::Keys::deKey ); + REQUIRE( !!lhcb ); + + CHECK( lhcb.interactionRegion().avgPosition == ROOT::Math::XYZPoint{0.0, 0.0, 0.0} ); + CHECK( lhcb.interactionRegion().spread.At( 0, 0 ) == 0.08 ); + CHECK( lhcb.interactionRegion().spread.At( 0, 1 ) == 0.00 ); + CHECK( lhcb.interactionRegion().spread.At( 0, 2 ) == 0.00 ); + CHECK( lhcb.interactionRegion().spread.At( 1, 1 ) == 0.08 ); + CHECK( lhcb.interactionRegion().spread.At( 1, 2 ) == 0.00 ); + CHECK( lhcb.interactionRegion().spread.At( 2, 2 ) == 53.0 ); + CHECK( lhcb.interactionRegion().tX == 0.0 ); + CHECK( lhcb.interactionRegion().tY == 0.0 ); + } +} -- GitLab From fb8f3fe813b4b0ec44366ca115d72c7cd7c1625c Mon Sep 17 00:00:00 2001 From: Thomas Latham Date: Fri, 9 Jun 2023 17:56:04 +0100 Subject: [PATCH 3/9] Load InteractionRegion condition - assumed a particular format for it --- Core/tests/src/test_DDS_interactionregion.cpp | 10 +++++++++- Detector/LHCb/src/DeLHCb.cpp | 6 +++++- Detector/LHCb/src/InteractionRegion.cpp | 15 ++++++++++++++- .../LHCb/Online/InteractionRegion.yml/.condition | 0 .../LHCb/Online/InteractionRegion.yml/0 | 3 +++ .../LHCb/Online/InteractionRegion.yml/200 | 10 ++++++++++ 6 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 tests/ConditionsIOV/Conditions/LHCb/Online/InteractionRegion.yml/.condition create mode 100644 tests/ConditionsIOV/Conditions/LHCb/Online/InteractionRegion.yml/0 create mode 100644 tests/ConditionsIOV/Conditions/LHCb/Online/InteractionRegion.yml/200 diff --git a/Core/tests/src/test_DDS_interactionregion.cpp b/Core/tests/src/test_DDS_interactionregion.cpp index 76b088c3bf..c2b9ed87af 100644 --- a/Core/tests/src/test_DDS_interactionregion.cpp +++ b/Core/tests/src/test_DDS_interactionregion.cpp @@ -46,7 +46,15 @@ TEST_CASE( "InteractionRegion loading" ) { LHCb::Detector::DeLHCb lhcb = slice->get( det, LHCb::Detector::Keys::deKey ); REQUIRE( !!lhcb ); - // TODO + CHECK( lhcb.interactionRegion().avgPosition == ROOT::Math::XYZPoint{0.0, 0.0, 0.0} ); + CHECK( lhcb.interactionRegion().spread.At( 0, 0 ) == 0.09 ); + CHECK( lhcb.interactionRegion().spread.At( 0, 1 ) == 0.00 ); + CHECK( lhcb.interactionRegion().spread.At( 0, 2 ) == 0.00 ); + CHECK( lhcb.interactionRegion().spread.At( 1, 1 ) == 0.09 ); + CHECK( lhcb.interactionRegion().spread.At( 1, 2 ) == 0.00 ); + CHECK( lhcb.interactionRegion().spread.At( 2, 2 ) == 52.0 ); + CHECK( lhcb.interactionRegion().tX == 0.0 ); + CHECK( lhcb.interactionRegion().tY == 0.0 ); } { diff --git a/Detector/LHCb/src/DeLHCb.cpp b/Detector/LHCb/src/DeLHCb.cpp index a858c1a9e5..36807a0344 100644 --- a/Detector/LHCb/src/DeLHCb.cpp +++ b/Detector/LHCb/src/DeLHCb.cpp @@ -117,7 +117,11 @@ void LHCb::Detector::setup_DeLHCb_callback( dd4hep::Detector& description ) { "Tell40Links" ); depbuilder.add( hash_key( de, "Tell40Links" ) ); - // TODO - add code here to register the InteractionRegion condition + // TODO - should this have something equivalent to iovUpdate->withTell40Links? What does that do? + ( *requests ) + ->addLocation( de, LHCb::Detector::item_key( "InteractionRegion" ), + "Conditions/LHCb/Online/InteractionRegion.yml", "InteractionRegion" ); + depbuilder.add( hash_key( de, "InteractionRegion" ) ); ( *requests )->addDependency( depbuilder.release() ); } diff --git a/Detector/LHCb/src/InteractionRegion.cpp b/Detector/LHCb/src/InteractionRegion.cpp index 949ddc14d8..945e92ba10 100644 --- a/Detector/LHCb/src/InteractionRegion.cpp +++ b/Detector/LHCb/src/InteractionRegion.cpp @@ -11,4 +11,17 @@ #include #include -LHCb::Detector::InteractionRegion::InteractionRegion( const nlohmann::json& /*obj*/ ) { throw NotImplemented{}; } +LHCb::Detector::InteractionRegion::InteractionRegion( const nlohmann::json& obj ) { + if ( !obj.contains( "position" ) || !obj.contains( "spread" ) ) { + throw std::runtime_error{"InteractionRegion condition is empty"}; + } + + auto& pos = obj.at( "position" ); + avgPosition.SetCoordinates( pos.begin(), pos.end() ); + + auto& spr = obj.at( "spread" ); + spread.SetElements( spr.begin(), spr.end(), true ); + + tX = spread( 0, 2 ) / spread( 2, 2 ); + tY = spread( 1, 2 ) / spread( 2, 2 ); +} diff --git a/tests/ConditionsIOV/Conditions/LHCb/Online/InteractionRegion.yml/.condition b/tests/ConditionsIOV/Conditions/LHCb/Online/InteractionRegion.yml/.condition new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/ConditionsIOV/Conditions/LHCb/Online/InteractionRegion.yml/0 b/tests/ConditionsIOV/Conditions/LHCb/Online/InteractionRegion.yml/0 new file mode 100644 index 0000000000..5099ee0e7f --- /dev/null +++ b/tests/ConditionsIOV/Conditions/LHCb/Online/InteractionRegion.yml/0 @@ -0,0 +1,3 @@ +# Special case of empty condition +--- +InteractionRegion: diff --git a/tests/ConditionsIOV/Conditions/LHCb/Online/InteractionRegion.yml/200 b/tests/ConditionsIOV/Conditions/LHCb/Online/InteractionRegion.yml/200 new file mode 100644 index 0000000000..66a8aab093 --- /dev/null +++ b/tests/ConditionsIOV/Conditions/LHCb/Online/InteractionRegion.yml/200 @@ -0,0 +1,10 @@ +# Example of InteractionRegion condition +# ``` +# InteractionRegion: +# position: +# spread: +# ``` +--- +InteractionRegion: + position: [ 0.0, 0.0, 0.0 ] + spread: [ 0.09, 0.0, 0.09, 0.0, 0.0, 52.0 ] -- GitLab From dea139736e8e4867a3a1695522b0c78b747c2edf Mon Sep 17 00:00:00 2001 From: Thomas Latham Date: Fri, 16 Jun 2023 11:46:49 +0100 Subject: [PATCH 4/9] Handle units in InteractionRegion condition --- .../include/Detector/LHCb/InteractionRegion.h | 2 +- Detector/LHCb/src/DeLHCb.cpp | 12 +++--- Detector/LHCb/src/InteractionRegion.cpp | 41 ++++++++++++++++--- .../LHCb/Online/InteractionRegion.yml/200 | 4 +- 4 files changed, 45 insertions(+), 14 deletions(-) diff --git a/Detector/LHCb/include/Detector/LHCb/InteractionRegion.h b/Detector/LHCb/include/Detector/LHCb/InteractionRegion.h index 96d1396f38..709abbedca 100644 --- a/Detector/LHCb/include/Detector/LHCb/InteractionRegion.h +++ b/Detector/LHCb/include/Detector/LHCb/InteractionRegion.h @@ -10,7 +10,7 @@ \*****************************************************************************/ #pragma once -#include +#include #include #include diff --git a/Detector/LHCb/src/DeLHCb.cpp b/Detector/LHCb/src/DeLHCb.cpp index 36807a0344..69199b3d73 100644 --- a/Detector/LHCb/src/DeLHCb.cpp +++ b/Detector/LHCb/src/DeLHCb.cpp @@ -30,14 +30,14 @@ LHCb::Detector::detail::DeLHCbObject::DeLHCbObject( const dd4hep::DetElement& // InteractionRegion condition doesn't exist, so need to fallback to other values // TODO - need to decide what these should be - using some trivial defaults for now // - can we access the DeVP::beamSpot to get the x and y positions? - m_interactionRegion.avgPosition = ROOT::Math::XYZPoint{0.0, 0.0, 0.0}; + m_interactionRegion.avgPosition = ROOT::Math::XYZPoint{0.0 * dd4hep::mm, 0.0 * dd4hep::mm, 0.0 * dd4hep::mm}; m_interactionRegion.spread = ROOT::Math::SMatrixSym3D{}; - m_interactionRegion.tX = 0.0; - m_interactionRegion.tY = 0.0; + m_interactionRegion.tX = 0.0 * dd4hep::mm; + m_interactionRegion.tY = 0.0 * dd4hep::mm; - m_interactionRegion.spread.At( 0, 0 ) = 0.08; - m_interactionRegion.spread.At( 1, 1 ) = 0.08; - m_interactionRegion.spread.At( 2, 2 ) = 53.0; + m_interactionRegion.spread.At( 0, 0 ) = 0.08 * dd4hep::mm; + m_interactionRegion.spread.At( 1, 1 ) = 0.08 * dd4hep::mm; + m_interactionRegion.spread.At( 2, 2 ) = 53.0 * dd4hep::mm; } } diff --git a/Detector/LHCb/src/InteractionRegion.cpp b/Detector/LHCb/src/InteractionRegion.cpp index 945e92ba10..d2ccfa427e 100644 --- a/Detector/LHCb/src/InteractionRegion.cpp +++ b/Detector/LHCb/src/InteractionRegion.cpp @@ -8,19 +8,50 @@ * granted to it by virtue of its status as an Intergovernmental Organization * * or submit itself to any jurisdiction. * \*****************************************************************************/ -#include #include +#include + +#include +#include + +#include + +template +std::array arrayFromJson( const nlohmann::json& jsonArray ) { + if ( jsonArray.size() != Size ) { throw std::runtime_error{"InteractionRegion condition array is wrong size"}; } + + std::array values; + + for ( std::size_t i{0}; i < Size; ++i ) { + if ( jsonArray[i].is_string() ) { + values[i] = dd4hep::_toDouble( jsonArray[i].get() ); + } else if ( jsonArray[i].is_number() ) { + values[i] = jsonArray[i].get() * dd4hep::mm; + } else { + throw std::runtime_error{"InteractionRegion condition contains unexpected types"}; + } + } + + return values; +} + LHCb::Detector::InteractionRegion::InteractionRegion( const nlohmann::json& obj ) { if ( !obj.contains( "position" ) || !obj.contains( "spread" ) ) { throw std::runtime_error{"InteractionRegion condition is empty"}; } - auto& pos = obj.at( "position" ); - avgPosition.SetCoordinates( pos.begin(), pos.end() ); + auto& pos = obj.at( "position" ); + constexpr std::size_t posSize{3}; + std::array posVals{arrayFromJson( pos )}; + + auto& spr = obj.at( "spread" ); + constexpr std::size_t sprSize{6}; + std::array sprVals{arrayFromJson( spr )}; + + avgPosition.SetCoordinates( posVals.begin(), posVals.end() ); - auto& spr = obj.at( "spread" ); - spread.SetElements( spr.begin(), spr.end(), true ); + spread.SetElements( sprVals.begin(), sprVals.end(), true ); tX = spread( 0, 2 ) / spread( 2, 2 ); tY = spread( 1, 2 ) / spread( 2, 2 ); diff --git a/tests/ConditionsIOV/Conditions/LHCb/Online/InteractionRegion.yml/200 b/tests/ConditionsIOV/Conditions/LHCb/Online/InteractionRegion.yml/200 index 66a8aab093..17cd4d9d29 100644 --- a/tests/ConditionsIOV/Conditions/LHCb/Online/InteractionRegion.yml/200 +++ b/tests/ConditionsIOV/Conditions/LHCb/Online/InteractionRegion.yml/200 @@ -6,5 +6,5 @@ # ``` --- InteractionRegion: - position: [ 0.0, 0.0, 0.0 ] - spread: [ 0.09, 0.0, 0.09, 0.0, 0.0, 52.0 ] + position: [ "0.0*mm", "0.0*mm", "0.0*mm" ] + spread: [ "0.09*mm", "0.0*mm", "0.09*mm", "0.0*mm", "0.0*mm", "52.0*mm" ] -- GitLab From 7e6e235260bbe154c29a0f3c8f6cfe7df1bdd19a Mon Sep 17 00:00:00 2001 From: Thomas Latham Date: Fri, 16 Jun 2023 12:01:55 +0100 Subject: [PATCH 5/9] Modify InteractionRegion to make tX and tY functions --- Core/tests/src/test_DDS_interactionregion.cpp | 8 ++++---- Detector/LHCb/include/Detector/LHCb/InteractionRegion.h | 5 +++-- Detector/LHCb/src/DeLHCb.cpp | 8 ++------ Detector/LHCb/src/InteractionRegion.cpp | 3 --- 4 files changed, 9 insertions(+), 15 deletions(-) diff --git a/Core/tests/src/test_DDS_interactionregion.cpp b/Core/tests/src/test_DDS_interactionregion.cpp index c2b9ed87af..5ec0c42822 100644 --- a/Core/tests/src/test_DDS_interactionregion.cpp +++ b/Core/tests/src/test_DDS_interactionregion.cpp @@ -53,8 +53,8 @@ TEST_CASE( "InteractionRegion loading" ) { CHECK( lhcb.interactionRegion().spread.At( 1, 1 ) == 0.09 ); CHECK( lhcb.interactionRegion().spread.At( 1, 2 ) == 0.00 ); CHECK( lhcb.interactionRegion().spread.At( 2, 2 ) == 52.0 ); - CHECK( lhcb.interactionRegion().tX == 0.0 ); - CHECK( lhcb.interactionRegion().tY == 0.0 ); + CHECK( lhcb.interactionRegion().tX() == 0.0 ); + CHECK( lhcb.interactionRegion().tY() == 0.0 ); } { @@ -73,7 +73,7 @@ TEST_CASE( "InteractionRegion loading" ) { CHECK( lhcb.interactionRegion().spread.At( 1, 1 ) == 0.08 ); CHECK( lhcb.interactionRegion().spread.At( 1, 2 ) == 0.00 ); CHECK( lhcb.interactionRegion().spread.At( 2, 2 ) == 53.0 ); - CHECK( lhcb.interactionRegion().tX == 0.0 ); - CHECK( lhcb.interactionRegion().tY == 0.0 ); + CHECK( lhcb.interactionRegion().tX() == 0.0 ); + CHECK( lhcb.interactionRegion().tY() == 0.0 ); } } diff --git a/Detector/LHCb/include/Detector/LHCb/InteractionRegion.h b/Detector/LHCb/include/Detector/LHCb/InteractionRegion.h index 709abbedca..a498f1b3a6 100644 --- a/Detector/LHCb/include/Detector/LHCb/InteractionRegion.h +++ b/Detector/LHCb/include/Detector/LHCb/InteractionRegion.h @@ -22,9 +22,10 @@ namespace LHCb::Detector { InteractionRegion() = default; InteractionRegion( const nlohmann::json& obj ); + double tX() const { return ( spread( 2, 2 ) != 0.0 ) ? spread( 0, 2 ) / spread( 2, 2 ) : 0.0; } + double tY() const { return ( spread( 2, 2 ) != 0.0 ) ? spread( 1, 2 ) / spread( 2, 2 ) : 0.0; } + ROOT::Math::XYZPoint avgPosition; ROOT::Math::SMatrixSym3D spread; - double tX; - double tY; }; } // namespace LHCb::Detector diff --git a/Detector/LHCb/src/DeLHCb.cpp b/Detector/LHCb/src/DeLHCb.cpp index 69199b3d73..f64f2d3fb0 100644 --- a/Detector/LHCb/src/DeLHCb.cpp +++ b/Detector/LHCb/src/DeLHCb.cpp @@ -27,13 +27,10 @@ LHCb::Detector::detail::DeLHCbObject::DeLHCbObject( const dd4hep::DetElement& try { m_interactionRegion = ctxt.condition( hash_key( de, "InteractionRegion" ) ).get(); } catch ( const std::runtime_error& e ) { - // InteractionRegion condition doesn't exist, so need to fallback to other values - // TODO - need to decide what these should be - using some trivial defaults for now - // - can we access the DeVP::beamSpot to get the x and y positions? + // InteractionRegion condition is empty, so need to fallback to other values + // TODO - need to finalise what these should be - using some trivial defaults for now m_interactionRegion.avgPosition = ROOT::Math::XYZPoint{0.0 * dd4hep::mm, 0.0 * dd4hep::mm, 0.0 * dd4hep::mm}; m_interactionRegion.spread = ROOT::Math::SMatrixSym3D{}; - m_interactionRegion.tX = 0.0 * dd4hep::mm; - m_interactionRegion.tY = 0.0 * dd4hep::mm; m_interactionRegion.spread.At( 0, 0 ) = 0.08 * dd4hep::mm; m_interactionRegion.spread.At( 1, 1 ) = 0.08 * dd4hep::mm; @@ -117,7 +114,6 @@ void LHCb::Detector::setup_DeLHCb_callback( dd4hep::Detector& description ) { "Tell40Links" ); depbuilder.add( hash_key( de, "Tell40Links" ) ); - // TODO - should this have something equivalent to iovUpdate->withTell40Links? What does that do? ( *requests ) ->addLocation( de, LHCb::Detector::item_key( "InteractionRegion" ), "Conditions/LHCb/Online/InteractionRegion.yml", "InteractionRegion" ); diff --git a/Detector/LHCb/src/InteractionRegion.cpp b/Detector/LHCb/src/InteractionRegion.cpp index d2ccfa427e..1cf6cdb706 100644 --- a/Detector/LHCb/src/InteractionRegion.cpp +++ b/Detector/LHCb/src/InteractionRegion.cpp @@ -52,7 +52,4 @@ LHCb::Detector::InteractionRegion::InteractionRegion( const nlohmann::json& obj avgPosition.SetCoordinates( posVals.begin(), posVals.end() ); spread.SetElements( sprVals.begin(), sprVals.end(), true ); - - tX = spread( 0, 2 ) / spread( 2, 2 ); - tY = spread( 1, 2 ) / spread( 2, 2 ); } -- GitLab From 8c8b72e17a72a72cd35ac53a48d2a47ff0a4b20c Mon Sep 17 00:00:00 2001 From: Thomas Latham Date: Fri, 16 Jun 2023 12:15:48 +0100 Subject: [PATCH 6/9] Make access to InteractionRegion via std::optional --- Core/tests/src/test_DDS_interactionregion.cpp | 32 ++++++++----------- Detector/LHCb/include/Detector/LHCb/DeLHCb.h | 5 +-- Detector/LHCb/src/DeLHCb.cpp | 10 ++---- 3 files changed, 19 insertions(+), 28 deletions(-) diff --git a/Core/tests/src/test_DDS_interactionregion.cpp b/Core/tests/src/test_DDS_interactionregion.cpp index 5ec0c42822..5f0d3f03c7 100644 --- a/Core/tests/src/test_DDS_interactionregion.cpp +++ b/Core/tests/src/test_DDS_interactionregion.cpp @@ -46,15 +46,18 @@ TEST_CASE( "InteractionRegion loading" ) { LHCb::Detector::DeLHCb lhcb = slice->get( det, LHCb::Detector::Keys::deKey ); REQUIRE( !!lhcb ); - CHECK( lhcb.interactionRegion().avgPosition == ROOT::Math::XYZPoint{0.0, 0.0, 0.0} ); - CHECK( lhcb.interactionRegion().spread.At( 0, 0 ) == 0.09 ); - CHECK( lhcb.interactionRegion().spread.At( 0, 1 ) == 0.00 ); - CHECK( lhcb.interactionRegion().spread.At( 0, 2 ) == 0.00 ); - CHECK( lhcb.interactionRegion().spread.At( 1, 1 ) == 0.09 ); - CHECK( lhcb.interactionRegion().spread.At( 1, 2 ) == 0.00 ); - CHECK( lhcb.interactionRegion().spread.At( 2, 2 ) == 52.0 ); - CHECK( lhcb.interactionRegion().tX() == 0.0 ); - CHECK( lhcb.interactionRegion().tY() == 0.0 ); + auto ir = lhcb.interactionRegion(); + REQUIRE( ir.has_value() ); + + CHECK( ir.value().avgPosition == ROOT::Math::XYZPoint{0.0, 0.0, 0.0} ); + CHECK( ir.value().spread.At( 0, 0 ) == 0.09 ); + CHECK( ir.value().spread.At( 0, 1 ) == 0.00 ); + CHECK( ir.value().spread.At( 0, 2 ) == 0.00 ); + CHECK( ir.value().spread.At( 1, 1 ) == 0.09 ); + CHECK( ir.value().spread.At( 1, 2 ) == 0.00 ); + CHECK( ir.value().spread.At( 2, 2 ) == 52.0 ); + CHECK( ir.value().tX() == 0.0 ); + CHECK( ir.value().tY() == 0.0 ); } { @@ -66,14 +69,7 @@ TEST_CASE( "InteractionRegion loading" ) { LHCb::Detector::DeLHCb lhcb = slice->get( det, LHCb::Detector::Keys::deKey ); REQUIRE( !!lhcb ); - CHECK( lhcb.interactionRegion().avgPosition == ROOT::Math::XYZPoint{0.0, 0.0, 0.0} ); - CHECK( lhcb.interactionRegion().spread.At( 0, 0 ) == 0.08 ); - CHECK( lhcb.interactionRegion().spread.At( 0, 1 ) == 0.00 ); - CHECK( lhcb.interactionRegion().spread.At( 0, 2 ) == 0.00 ); - CHECK( lhcb.interactionRegion().spread.At( 1, 1 ) == 0.08 ); - CHECK( lhcb.interactionRegion().spread.At( 1, 2 ) == 0.00 ); - CHECK( lhcb.interactionRegion().spread.At( 2, 2 ) == 53.0 ); - CHECK( lhcb.interactionRegion().tX() == 0.0 ); - CHECK( lhcb.interactionRegion().tY() == 0.0 ); + auto ir = lhcb.interactionRegion(); + REQUIRE( !ir.has_value() ); } } diff --git a/Detector/LHCb/include/Detector/LHCb/DeLHCb.h b/Detector/LHCb/include/Detector/LHCb/DeLHCb.h index e7c901129d..e942f3ebff 100644 --- a/Detector/LHCb/include/Detector/LHCb/DeLHCb.h +++ b/Detector/LHCb/include/Detector/LHCb/DeLHCb.h @@ -19,6 +19,7 @@ #include #include +#include namespace LHCb::Detector { @@ -29,7 +30,7 @@ namespace LHCb::Detector { void applyToAllChildren( const std::function )>& ) const override; std::vector children; Tell40Links m_tell40links; - InteractionRegion m_interactionRegion; + std::optional m_interactionRegion; }; // Utility method to lookup DeIOV object from the condition slice @@ -44,7 +45,7 @@ namespace LHCb::Detector { auto tell40links( Tell40Links::SourceId id ) const { return access()->m_tell40links[id]; } /// Provide the position, spread, and tilt of the interaction region - const InteractionRegion& interactionRegion() const { return this->access()->m_interactionRegion; } + std::optional interactionRegion() const { return this->access()->m_interactionRegion; } }; // Utility method to setup DeLHCb diff --git a/Detector/LHCb/src/DeLHCb.cpp b/Detector/LHCb/src/DeLHCb.cpp index f64f2d3fb0..2c2aec4c60 100644 --- a/Detector/LHCb/src/DeLHCb.cpp +++ b/Detector/LHCb/src/DeLHCb.cpp @@ -27,14 +27,8 @@ LHCb::Detector::detail::DeLHCbObject::DeLHCbObject( const dd4hep::DetElement& try { m_interactionRegion = ctxt.condition( hash_key( de, "InteractionRegion" ) ).get(); } catch ( const std::runtime_error& e ) { - // InteractionRegion condition is empty, so need to fallback to other values - // TODO - need to finalise what these should be - using some trivial defaults for now - m_interactionRegion.avgPosition = ROOT::Math::XYZPoint{0.0 * dd4hep::mm, 0.0 * dd4hep::mm, 0.0 * dd4hep::mm}; - m_interactionRegion.spread = ROOT::Math::SMatrixSym3D{}; - - m_interactionRegion.spread.At( 0, 0 ) = 0.08 * dd4hep::mm; - m_interactionRegion.spread.At( 1, 1 ) = 0.08 * dd4hep::mm; - m_interactionRegion.spread.At( 2, 2 ) = 53.0 * dd4hep::mm; + // InteractionRegion condition is empty + // Nothing to do - the optional will not contain a value } } -- GitLab From c54d94a8cc5007b5d03783e030a5da9dac1b694d Mon Sep 17 00:00:00 2001 From: Thomas Latham Date: Fri, 16 Jun 2023 14:01:59 +0100 Subject: [PATCH 7/9] Fix values and units of the spread matrix --- Core/tests/src/test_DDS_interactionregion.cpp | 12 ++++++------ Detector/LHCb/src/InteractionRegion.cpp | 8 ++++---- .../Conditions/LHCb/Online/InteractionRegion.yml/200 | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Core/tests/src/test_DDS_interactionregion.cpp b/Core/tests/src/test_DDS_interactionregion.cpp index 5f0d3f03c7..aa57581068 100644 --- a/Core/tests/src/test_DDS_interactionregion.cpp +++ b/Core/tests/src/test_DDS_interactionregion.cpp @@ -50,12 +50,12 @@ TEST_CASE( "InteractionRegion loading" ) { REQUIRE( ir.has_value() ); CHECK( ir.value().avgPosition == ROOT::Math::XYZPoint{0.0, 0.0, 0.0} ); - CHECK( ir.value().spread.At( 0, 0 ) == 0.09 ); - CHECK( ir.value().spread.At( 0, 1 ) == 0.00 ); - CHECK( ir.value().spread.At( 0, 2 ) == 0.00 ); - CHECK( ir.value().spread.At( 1, 1 ) == 0.09 ); - CHECK( ir.value().spread.At( 1, 2 ) == 0.00 ); - CHECK( ir.value().spread.At( 2, 2 ) == 52.0 ); + CHECK( ir.value().spread.At( 0, 0 ) == 0.0064 ); + CHECK( ir.value().spread.At( 0, 1 ) == 0.0 ); + CHECK( ir.value().spread.At( 0, 2 ) == 0.0 ); + CHECK( ir.value().spread.At( 1, 1 ) == 0.0064 ); + CHECK( ir.value().spread.At( 1, 2 ) == 0.0 ); + CHECK( ir.value().spread.At( 2, 2 ) == 2809.0 ); CHECK( ir.value().tX() == 0.0 ); CHECK( ir.value().tY() == 0.0 ); } diff --git a/Detector/LHCb/src/InteractionRegion.cpp b/Detector/LHCb/src/InteractionRegion.cpp index 1cf6cdb706..adf856311b 100644 --- a/Detector/LHCb/src/InteractionRegion.cpp +++ b/Detector/LHCb/src/InteractionRegion.cpp @@ -18,7 +18,7 @@ #include template -std::array arrayFromJson( const nlohmann::json& jsonArray ) { +std::array arrayFromJson( const nlohmann::json& jsonArray, const double unit ) { if ( jsonArray.size() != Size ) { throw std::runtime_error{"InteractionRegion condition array is wrong size"}; } std::array values; @@ -27,7 +27,7 @@ std::array arrayFromJson( const nlohmann::json& jsonArray ) { if ( jsonArray[i].is_string() ) { values[i] = dd4hep::_toDouble( jsonArray[i].get() ); } else if ( jsonArray[i].is_number() ) { - values[i] = jsonArray[i].get() * dd4hep::mm; + values[i] = jsonArray[i].get() * unit; } else { throw std::runtime_error{"InteractionRegion condition contains unexpected types"}; } @@ -43,11 +43,11 @@ LHCb::Detector::InteractionRegion::InteractionRegion( const nlohmann::json& obj auto& pos = obj.at( "position" ); constexpr std::size_t posSize{3}; - std::array posVals{arrayFromJson( pos )}; + std::array posVals{arrayFromJson( pos, dd4hep::mm )}; auto& spr = obj.at( "spread" ); constexpr std::size_t sprSize{6}; - std::array sprVals{arrayFromJson( spr )}; + std::array sprVals{arrayFromJson( spr, dd4hep::mm2 )}; avgPosition.SetCoordinates( posVals.begin(), posVals.end() ); diff --git a/tests/ConditionsIOV/Conditions/LHCb/Online/InteractionRegion.yml/200 b/tests/ConditionsIOV/Conditions/LHCb/Online/InteractionRegion.yml/200 index 17cd4d9d29..0b0cfa96fa 100644 --- a/tests/ConditionsIOV/Conditions/LHCb/Online/InteractionRegion.yml/200 +++ b/tests/ConditionsIOV/Conditions/LHCb/Online/InteractionRegion.yml/200 @@ -7,4 +7,4 @@ --- InteractionRegion: position: [ "0.0*mm", "0.0*mm", "0.0*mm" ] - spread: [ "0.09*mm", "0.0*mm", "0.09*mm", "0.0*mm", "0.0*mm", "52.0*mm" ] + spread: [ "0.0064*mm2", "0.0*mm2", "0.0064*mm2", "0.0*mm2", "0.0*mm2", "2809.0*mm2" ] -- GitLab From f76483a05f3abce73c4adbe98b376ad8bddf2190 Mon Sep 17 00:00:00 2001 From: Thomas Latham Date: Fri, 16 Jun 2023 15:24:10 +0100 Subject: [PATCH 8/9] Allow protection against InteractionRegion condition not existing in DB --- Detector/LHCb/src/DeLHCb.cpp | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/Detector/LHCb/src/DeLHCb.cpp b/Detector/LHCb/src/DeLHCb.cpp index 2c2aec4c60..df534d1df3 100644 --- a/Detector/LHCb/src/DeLHCb.cpp +++ b/Detector/LHCb/src/DeLHCb.cpp @@ -24,11 +24,13 @@ LHCb::Detector::detail::DeLHCbObject::DeLHCbObject( const dd4hep::DetElement& auto links = ctxt.condition( hash_key( de, "Tell40Links" ) ).get(); if ( !links.is_null() ) { m_tell40links = links; } - try { - m_interactionRegion = ctxt.condition( hash_key( de, "InteractionRegion" ) ).get(); - } catch ( const std::runtime_error& e ) { - // InteractionRegion condition is empty - // Nothing to do - the optional will not contain a value + if ( withInteractionRegion ) { + try { + m_interactionRegion = ctxt.condition( hash_key( de, "InteractionRegion" ) ).get(); + } catch ( const std::runtime_error& e ) { + // InteractionRegion condition is empty + // Nothing to do - the optional will not contain a value + } } } @@ -108,10 +110,14 @@ void LHCb::Detector::setup_DeLHCb_callback( dd4hep::Detector& description ) { "Tell40Links" ); depbuilder.add( hash_key( de, "Tell40Links" ) ); - ( *requests ) - ->addLocation( de, LHCb::Detector::item_key( "InteractionRegion" ), - "Conditions/LHCb/Online/InteractionRegion.yml", "InteractionRegion" ); - depbuilder.add( hash_key( de, "InteractionRegion" ) ); + iovUpdate->withInteractionRegion = + opts && !( opts->contains( "without_interactionregion" ) && opts->at( "without_interactionregion" ).get() ); + if ( iovUpdate->withInteractionRegion ) { + ( *requests ) + ->addLocation( de, LHCb::Detector::item_key( "InteractionRegion" ), + "Conditions/LHCb/Online/InteractionRegion.yml", "InteractionRegion" ); + depbuilder.add( hash_key( de, "InteractionRegion" ) ); + } ( *requests )->addDependency( depbuilder.release() ); } -- GitLab From dcffa1e82c502acf8ad38e6012880b7250fe143b Mon Sep 17 00:00:00 2001 From: Roel Aaij Date: Mon, 24 Jul 2023 16:19:55 +0200 Subject: [PATCH 9/9] Use test for is_null instead of exception --- Detector/LHCb/src/DeLHCb.cpp | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/Detector/LHCb/src/DeLHCb.cpp b/Detector/LHCb/src/DeLHCb.cpp index df534d1df3..1aed644202 100644 --- a/Detector/LHCb/src/DeLHCb.cpp +++ b/Detector/LHCb/src/DeLHCb.cpp @@ -24,14 +24,8 @@ LHCb::Detector::detail::DeLHCbObject::DeLHCbObject( const dd4hep::DetElement& auto links = ctxt.condition( hash_key( de, "Tell40Links" ) ).get(); if ( !links.is_null() ) { m_tell40links = links; } - if ( withInteractionRegion ) { - try { - m_interactionRegion = ctxt.condition( hash_key( de, "InteractionRegion" ) ).get(); - } catch ( const std::runtime_error& e ) { - // InteractionRegion condition is empty - // Nothing to do - the optional will not contain a value - } - } + auto cond = ctxt.condition( hash_key( de, "InteractionRegion" ) ).get(); + if ( !cond.is_null() ) { m_interactionRegion = cond; } } void LHCb::Detector::detail::DeLHCbObject::applyToAllChildren( @@ -110,14 +104,10 @@ void LHCb::Detector::setup_DeLHCb_callback( dd4hep::Detector& description ) { "Tell40Links" ); depbuilder.add( hash_key( de, "Tell40Links" ) ); - iovUpdate->withInteractionRegion = - opts && !( opts->contains( "without_interactionregion" ) && opts->at( "without_interactionregion" ).get() ); - if ( iovUpdate->withInteractionRegion ) { - ( *requests ) - ->addLocation( de, LHCb::Detector::item_key( "InteractionRegion" ), - "Conditions/LHCb/Online/InteractionRegion.yml", "InteractionRegion" ); - depbuilder.add( hash_key( de, "InteractionRegion" ) ); - } + ( *requests ) + ->addLocation( de, LHCb::Detector::item_key( "InteractionRegion" ), + "Conditions/LHCb/Online/InteractionRegion.yml", "InteractionRegion" ); + depbuilder.add( hash_key( de, "InteractionRegion" ) ); ( *requests )->addDependency( depbuilder.release() ); } -- GitLab