From 0c05e4d8c81b0d190728a4249aa9e9733647c998 Mon Sep 17 00:00:00 2001 From: Marco Clemencic Date: Wed, 17 Sep 2025 16:51:46 +0200 Subject: [PATCH 1/4] Replace the UT parameter WithCorrectedP0Parameter with the condition UseCorrectP0Parameter This allows detecting the way the alignment was produced and use the matching p0 parameter computation. For backward compatibility, if the condition is missing it is considered false and we use the legacy, wrong p0 parameter. --- Detector/UT/include/Detector/UT/DeUTSensor.h | 4 ++- Detector/UT/src/DeUT.cpp | 35 ++++++++++++++++--- .../components/UT/2024-v00.00/parameters.xml | 4 --- .../components/UT/2024-v00.01/parameters.xml | 4 --- .../components/UT/2024-v00.02/parameters.xml | 4 --- compact/components/UT/trunk/conditions.xml | 4 +++ compact/components/UT/trunk/parameters.xml | 4 --- tests/ConditionsIOV/.schema.json | 3 ++ .../Conditions/UT/UseCorrectP0Parameter.yml | 3 ++ 9 files changed, 43 insertions(+), 22 deletions(-) create mode 100644 tests/ConditionsIOV/Conditions/UT/UseCorrectP0Parameter.yml diff --git a/Detector/UT/include/Detector/UT/DeUTSensor.h b/Detector/UT/include/Detector/UT/DeUTSensor.h index 4e07d3e697..650d700be1 100644 --- a/Detector/UT/include/Detector/UT/DeUTSensor.h +++ b/Detector/UT/include/Detector/UT/DeUTSensor.h @@ -45,6 +45,8 @@ namespace LHCb::Detector::UT { bool m_xInverted = false; bool m_yInverted = false; + bool m_useCorrectP0Parameter = false; + ROOT::Math::Plane3D m_plane; ROOT::Math::Plane3D m_entryPlane; ROOT::Math::Plane3D m_exitPlane; @@ -97,7 +99,7 @@ namespace LHCb::Detector::UT { return ( strip >= firstStrip() ) && strip < nStrip() + firstStrip(); } LineTraj trajectory( unsigned int strip, double offset ) const { - if ( dd4hep::_toInt( "WithCorrectedP0Parameter" ) && !isStrip( strip ) ) { + if ( this->access()->m_useCorrectP0Parameter && !isStrip( strip ) ) { throw std::out_of_range( fmt::format( "DeUTSector::createTraj: strip out of range, given {strip}, expect [{first}, {last}]", fmt::arg( "strip", strip ), fmt::arg( "first", firstStrip() ), diff --git a/Detector/UT/src/DeUT.cpp b/Detector/UT/src/DeUT.cpp index 782baa9c44..1bffd5a2c1 100644 --- a/Detector/UT/src/DeUT.cpp +++ b/Detector/UT/src/DeUT.cpp @@ -12,10 +12,30 @@ #include "Detector/UT/DeUT.h" #include "Detector/UT/UTConstants.h" +#include #include #include #include +namespace { + /// Precomputed the UseCorrectP0Parameter condition key (hash) + static const dd4hep::Condition::key_type USE_CORRECT_P0_PARAMETER_CONDKEY = + dd4hep::ConditionKey::KeyMaker( dd4hep::detail::hash32( "/world/BeforeMagnetRegion/UT" ), + "UseCorrectP0Parameter" ) + .hash; + + /// extract the UseCorrectP0Parameter flag from the condition context + /// + /// If the condition is null or missing, return false for backward compatibility. + bool useCorrectP0Parameter( dd4hep::cond::ConditionUpdateContext& ctxt ) { + if ( auto cond = ctxt.condition( USE_CORRECT_P0_PARAMETER_CONDKEY, false ); cond.isValid() ) { + auto const& value = cond.get(); + return value.is_null() ? false : value.get(); + } + return false; + } +} // namespace + LHCb::Detector::UT::Status LHCb::Detector::UT::toStatus( std::string_view str ) { auto i = std::find_if( LHCb::Detector::UT::s_map.begin(), LHCb::Detector::UT::s_map.end(), [&]( const auto& i ) { return i.second == str; } ); @@ -54,6 +74,11 @@ LHCb::Detector::UT::detail::DeUTObject::DeUTObject( const dd4hep::DetElement& dd4hep::cond::ConditionUpdateContext& ctxt ) : DeIOVObject( de, ctxt, 9301 ), m_sides{ { { de.child( "Cside" ), ctxt }, { de.child( "Aside" ), ctxt } } } { m_sectors.reserve( 2000 ); + + // this is just for information, done here to avoid printing one message for each sector + dd4hep::printout( dd4hep::DEBUG, "UT::cacheInfo", + useCorrectP0Parameter( ctxt ) ? "With correct p0 parameter" : "With wrong p0 parameter" ); + // flatten sectors applyToAllSectors( [this]( DeUTSector s ) { m_sectors.emplace_back( &*s ); } ); } @@ -239,13 +264,11 @@ LHCb::Detector::UT::detail::DeUTSectorObject::DeUTSectorObject( const dd4hep::De auto thisSector = DeUTSector{ this }; auto firstTraj = thisSector.trajectoryFirstStrip(); if ( thisSector.stripflip() && thisSector.xInverted() ) { - if ( dd4hep::_toInt( "WithCorrectedP0Parameter" ) ) { - // use CORRECT p0 parameter since 2025 data taking (>2024-v00.00) - dd4hep::printout( dd4hep::DEBUG, "UT::cacheInfo", "With correct p0 parameter" ); + if ( useCorrectP0Parameter( ctxt ) ) { + // use CORRECT p0 parameter firstTraj = thisSector.trajectoryLastStrip(); } else { - // use WRONG p0 parameter before 2025 data taking (<=2024-v00.00) - dd4hep::printout( dd4hep::DEBUG, "UT::cacheInfo", "With wrong p0 parameter" ); + // use WRONG p0 parameter firstTraj = thisSector.createTraj( thisSector.nStrip() + thisSector.firstStrip(), 0 ); } } @@ -505,6 +528,8 @@ LHCb::Detector::UT::detail::DeUTSensorObject::DeUTSensorObject( const dd4hep::De m_yInverted = ( g1.y() > g3.y() ); } + m_useCorrectP0Parameter = useCorrectP0Parameter( ctxt ); + { float yUpper = m_vMaxLocal; float yLower = m_vMinLocal; diff --git a/compact/components/UT/2024-v00.00/parameters.xml b/compact/components/UT/2024-v00.00/parameters.xml index 975c1670eb..3c30e6ccb3 100644 --- a/compact/components/UT/2024-v00.00/parameters.xml +++ b/compact/components/UT/2024-v00.00/parameters.xml @@ -17,10 +17,6 @@ - - - - diff --git a/compact/components/UT/2024-v00.01/parameters.xml b/compact/components/UT/2024-v00.01/parameters.xml index 647c67deba..3c30e6ccb3 100644 --- a/compact/components/UT/2024-v00.01/parameters.xml +++ b/compact/components/UT/2024-v00.01/parameters.xml @@ -17,10 +17,6 @@ - - - - diff --git a/compact/components/UT/2024-v00.02/parameters.xml b/compact/components/UT/2024-v00.02/parameters.xml index 7d976324fd..ec30eddb29 100644 --- a/compact/components/UT/2024-v00.02/parameters.xml +++ b/compact/components/UT/2024-v00.02/parameters.xml @@ -17,10 +17,6 @@ - - - - diff --git a/compact/components/UT/trunk/conditions.xml b/compact/components/UT/trunk/conditions.xml index 0250992e50..7ee808d202 100644 --- a/compact/components/UT/trunk/conditions.xml +++ b/compact/components/UT/trunk/conditions.xml @@ -63,6 +63,10 @@ + + + + diff --git a/compact/components/UT/trunk/parameters.xml b/compact/components/UT/trunk/parameters.xml index 7d976324fd..ec30eddb29 100644 --- a/compact/components/UT/trunk/parameters.xml +++ b/compact/components/UT/trunk/parameters.xml @@ -17,10 +17,6 @@ - - - - diff --git a/tests/ConditionsIOV/.schema.json b/tests/ConditionsIOV/.schema.json index 8c6fc0c82e..9deec77a81 100644 --- a/tests/ConditionsIOV/.schema.json +++ b/tests/ConditionsIOV/.schema.json @@ -3946,6 +3946,9 @@ "Conditions/UT/ReadoutConf/ProductionMap.yml": [ "ProductionMap" ], + "Conditions/UT/UseCorrectP0Parameter.yml": [ + "UseCorrectP0Parameter" + ], "Conditions/VP/Alignment/Global.yml": [ "VPLeft", "VPRight", diff --git a/tests/ConditionsIOV/Conditions/UT/UseCorrectP0Parameter.yml b/tests/ConditionsIOV/Conditions/UT/UseCorrectP0Parameter.yml new file mode 100644 index 0000000000..c7f1d816b7 --- /dev/null +++ b/tests/ConditionsIOV/Conditions/UT/UseCorrectP0Parameter.yml @@ -0,0 +1,3 @@ +# This flags alignments produced with the correct value of the p0 parameter +# (Detector > 3.5 or Detector > 3.3.3) +UseCorrectP0Parameter: true -- GitLab From cb80907530753f36810f6bbe4e5d97e224261511 Mon Sep 17 00:00:00 2001 From: Marco Clemencic Date: Thu, 18 Sep 2025 14:43:53 +0200 Subject: [PATCH 2/4] Add test for UseCorrectP0Parameter parsing logic --- CMakeLists.txt | 1 + Detector/UT/src/DeUT.cpp | 16 ++++-- Detector/UT/tests/CMakeLists.txt | 32 ++++++++++++ .../UT/tests/src/test_p0_parameter_switch.cpp | 52 +++++++++++++++++++ 4 files changed, 98 insertions(+), 3 deletions(-) create mode 100644 Detector/UT/tests/CMakeLists.txt create mode 100644 Detector/UT/tests/src/test_p0_parameter_switch.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 0e534d2386..b648380a38 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -124,6 +124,7 @@ if(BUILD_TESTING) add_subdirectory(Detector/Calo/tests) add_subdirectory(Detector/Muon/tests) add_subdirectory(Detector/LHCb/tests) + add_subdirectory(Detector/UT/tests) #add_test (NAME MyTest COMMAND Test) # Prepare the Detector tests Plugins diff --git a/Detector/UT/src/DeUT.cpp b/Detector/UT/src/DeUT.cpp index 1bffd5a2c1..ef93413bf1 100644 --- a/Detector/UT/src/DeUT.cpp +++ b/Detector/UT/src/DeUT.cpp @@ -24,16 +24,26 @@ namespace { "UseCorrectP0Parameter" ) .hash; - /// extract the UseCorrectP0Parameter flag from the condition context + /// extract the UseCorrectP0Parameter condition from the condition context + dd4hep::Condition getCorrectP0ParameterCondition( dd4hep::cond::ConditionUpdateContext& ctxt ) { + return ctxt.condition( USE_CORRECT_P0_PARAMETER_CONDKEY, false ); + } + + /// convert UseCorrectP0Parameter condition to bool /// /// If the condition is null or missing, return false for backward compatibility. - bool useCorrectP0Parameter( dd4hep::cond::ConditionUpdateContext& ctxt ) { - if ( auto cond = ctxt.condition( USE_CORRECT_P0_PARAMETER_CONDKEY, false ); cond.isValid() ) { + bool useCorrectP0Parameter( dd4hep::Condition cond ) { + if ( cond.isValid() ) { auto const& value = cond.get(); return value.is_null() ? false : value.get(); } return false; } + + /// helper to extract value of UseCorrectP0Parameter from the condition context + bool useCorrectP0Parameter( dd4hep::cond::ConditionUpdateContext& ctxt ) { + return useCorrectP0Parameter( getCorrectP0ParameterCondition( ctxt ) ); + } } // namespace LHCb::Detector::UT::Status LHCb::Detector::UT::toStatus( std::string_view str ) { diff --git a/Detector/UT/tests/CMakeLists.txt b/Detector/UT/tests/CMakeLists.txt new file mode 100644 index 0000000000..33c6266659 --- /dev/null +++ b/Detector/UT/tests/CMakeLists.txt @@ -0,0 +1,32 @@ +############################################################################### +# (c) Copyright 2025 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. # +############################################################################### + +# Tests not requiring DetectorPlugins or DD4hep +add_executable(ut_code_tests + src/test_p0_parameter_switch.cpp +) +target_link_libraries(ut_code_tests + Catch2::Catch2WithMain + fmt::fmt + nlohmann_json::nlohmann_json + yaml-cpp + DetectorCore + DetectorLHCb + DetectorUT +) +target_link_options(ut_code_tests PRIVATE -Wl,-allow-multiple-definition) + +catch_discover_tests(ut_code_tests + TEST_PREFIX Detector.UT. + PROPERTIES + LABELS "Detector" + LABELS "Detector.UT" +) diff --git a/Detector/UT/tests/src/test_p0_parameter_switch.cpp b/Detector/UT/tests/src/test_p0_parameter_switch.cpp new file mode 100644 index 0000000000..53d6e71c6b --- /dev/null +++ b/Detector/UT/tests/src/test_p0_parameter_switch.cpp @@ -0,0 +1,52 @@ +/*****************************************************************************\ +* (c) Copyright 2025 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 + +// this is to have access to a static method in DeUT.cpp +// (note that we have to link with --allow-multiple-definition because we are +// duplicating symbols already defined in library DetectorUT) +#include "../../src/DeUT.cpp" + +#if __has_include( ) +// Catch2 v2 +# include +#else +// Catch2 v3 +# include +#endif + +namespace { + // helper to create a condition object from json + dd4hep::Condition make_condition( nlohmann::json json = {} ) { + dd4hep::Condition cond{ std::string{ "UseCorrectP0Parameter" }, "" }; + auto& payload = cond.bind(); + payload = json; + return cond; + } +} // namespace + +TEST_CASE( "UseCorrectP0Parameter conversion logic" ) { + SECTION( "invalid condition" ) { + auto cond = make_condition(); + CHECK( useCorrectP0Parameter( cond ) == false ); + } + SECTION( "null condition" ) { + auto cond = make_condition(); + CHECK( useCorrectP0Parameter( cond ) == false ); + } + SECTION( "actual values" ) { + auto cond_true = make_condition( true ); + auto cond_false = make_condition( false ); + + CHECK( useCorrectP0Parameter( cond_true ) == true ); + CHECK( useCorrectP0Parameter( cond_false ) == false ); + } +} -- GitLab From 8ffdab6a700add742821bb0b6f4a4ba6cbe1e513 Mon Sep 17 00:00:00 2001 From: Marco Clemencic Date: Thu, 18 Sep 2025 16:06:41 +0200 Subject: [PATCH 3/4] Add test to ensure that UseCorrectP0Parameter condition is used correctly --- Detector/UT/tests/CMakeLists.txt | 22 ++++++++ Detector/UT/tests/src/test_DeUT.cpp | 82 +++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+) create mode 100644 Detector/UT/tests/src/test_DeUT.cpp diff --git a/Detector/UT/tests/CMakeLists.txt b/Detector/UT/tests/CMakeLists.txt index 33c6266659..89a5799755 100644 --- a/Detector/UT/tests/CMakeLists.txt +++ b/Detector/UT/tests/CMakeLists.txt @@ -30,3 +30,25 @@ catch_discover_tests(ut_code_tests LABELS "Detector" LABELS "Detector.UT" ) + +# Tests requiring DetectorPlugins and DD4hep +add_executable(ut_tests + src/test_DeUT.cpp +) +target_link_libraries(ut_tests + Catch2::Catch2WithMain + DetectorLib + TestUtils +) +# FIXME: this is too specific to the way lcg-toolchains and Gaudi projects work +set_target_properties(ut_tests PROPERTIES CROSSCOMPILING_EMULATOR ${PROJECT_BINARY_DIR}/bin/run) +# This is to ensure that we have everything we need if we want to build and run only this test +add_dependencies(ut_tests DetectorPlugins) + +catch_discover_tests(ut_tests + TEST_PREFIX Detector.UT. + WORKING_DIRECTORY ${PROJECT_SOURCE_DIR} + PROPERTIES + LABELS "Detector" + LABELS "Detector.UT" +) diff --git a/Detector/UT/tests/src/test_DeUT.cpp b/Detector/UT/tests/src/test_DeUT.cpp new file mode 100644 index 0000000000..de251cb3b4 --- /dev/null +++ b/Detector/UT/tests/src/test_DeUT.cpp @@ -0,0 +1,82 @@ +/*****************************************************************************\ +* (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 + +#if __has_include( ) +// Catch2 v2 +# include +namespace Catch { + using Detail::Approx; +} +#else +// Catch2 v3 +# include +#endif + +TEST_CASE( "DeUT" ) { + Detector::Test::Fixture f; + + auto& description = f.description(); + + description.fromXML( "compact/components/debug/UT.xml" ); + + REQUIRE( description.state() == dd4hep::Detector::READY ); + + auto ut = description.detector( "UT" ); + + // the `!!` is needed because handles have `operator!` but not `operator bool` + REQUIRE( !!ut ); + + // load conditions + LHCb::Detector::DetectorDataService dds( description, { "/world", "UT" } ); + dds.initialize( nlohmann::json{ { "repository", "file:tests/ConditionsIOV" }, { "overlay", true } } ); + + SECTION( "use correct p0 computation" ) { + auto slice = dds.get_slice( 100 ); + REQUIRE( slice ); + + using LHCb::Detector::UT::DeUT; + DeUT de = slice->get( ut, LHCb::Detector::Keys::deKey ); + REQUIRE( !!de ); + + // empirically, sector 6 is affected by the wrong p0 computation + auto sector = de.sector( 6 ); + REQUIRE( !!sector ); + auto sensor = sector.sensor(); + REQUIRE( !!sensor ); + CHECK( sensor.access()->m_useCorrectP0Parameter == true ); + CHECK( sector.access()->m_p0.x() == Approx( -46.9143 ) ); + } + + SECTION( "use wrong p0 computation" ) { + dds.update_condition( "Conditions/UT/UseCorrectP0Parameter.yml", "UseCorrectP0Parameter", "false" ); + auto slice = dds.get_slice( 100 ); + REQUIRE( slice ); + + using LHCb::Detector::UT::DeUT; + DeUT de = slice->get( ut, LHCb::Detector::Keys::deKey ); + REQUIRE( !!de ); + + // empirically, sector 6 is affected by the wrong p0 computation + auto sector = de.sector( 6 ); + REQUIRE( !!sector ); + auto sensor = sector.sensor(); + REQUIRE( !!sensor ); + CHECK( sensor.access()->m_useCorrectP0Parameter == false ); + CHECK( sector.access()->m_p0.x() == Approx( -47.0078 ) ); + } +} -- GitLab From ed8e21d8f9eae70ba1138ad542ba5873e1abfbfc Mon Sep 17 00:00:00 2001 From: Arthur Hennequin Date: Fri, 19 Sep 2025 10:35:34 +0200 Subject: [PATCH 4/4] Fix overlay update when data is empty --- Core/include/Core/ConditionsOverlay.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Core/include/Core/ConditionsOverlay.h b/Core/include/Core/ConditionsOverlay.h index e2c01be12e..a26b6824c0 100644 --- a/Core/include/Core/ConditionsOverlay.h +++ b/Core/include/Core/ConditionsOverlay.h @@ -83,6 +83,8 @@ namespace LHCb::Detector { // If the tree contains multiple documents, we update the first one: if ( root.is_stream() ) { root = root.child( 0 ); } + root |= ryml::MAP; // mark root as a map + auto node = root[name.c_str()]; // Clear previous condition, if it already existed, to empty the node: -- GitLab