From 6d69f5f35b67dd7e4c8009c000f26579b4bb8092 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 461400c57b..88c5288dfa 100644 --- a/tests/ConditionsIOV/.schema.json +++ b/tests/ConditionsIOV/.schema.json @@ -3877,6 +3877,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 19eefe39359f46cbc0a51b414129b070180c44ce 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 318d6e5077..2585b646ee 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -122,6 +122,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 c6a5ad56b46874aaf5049787dc1e94b87e8822da 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 0166492f2d93b8a5330bc19c22d846201e3c092b 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