diff --git a/CMakeLists.txt b/CMakeLists.txt index 0e534d2386088925868f6a412d87993957f5f2d0..b648380a38f2e4f5b47e4531182c6aa449c55332 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/Core/include/Core/ConditionsOverlay.h b/Core/include/Core/ConditionsOverlay.h index e2c01be12e61edd1c5d40132fddef6fc13c88478..a26b6824c0c2d1211760039adee035d0bbd581c1 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: diff --git a/Detector/UT/include/Detector/UT/DeUTSensor.h b/Detector/UT/include/Detector/UT/DeUTSensor.h index 4e07d3e69707e53678baa7d9974b9d9e97758187..650d700be12d117f2c2468e2c07e0b464a48a1cf 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 782baa9c44ec7d544da2fdbe64a012cdeecf2d44..ef93413bf1d73c74a2394f48aad7b700888f6e98 100644 --- a/Detector/UT/src/DeUT.cpp +++ b/Detector/UT/src/DeUT.cpp @@ -12,10 +12,40 @@ #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 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::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 ) { 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 +84,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 +274,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 +538,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/Detector/UT/tests/CMakeLists.txt b/Detector/UT/tests/CMakeLists.txt new file mode 100644 index 0000000000000000000000000000000000000000..89a57997555cb6dc0a9326ee0c8806d353a210b0 --- /dev/null +++ b/Detector/UT/tests/CMakeLists.txt @@ -0,0 +1,54 @@ +############################################################################### +# (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" +) + +# 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 0000000000000000000000000000000000000000..de251cb3b415959cfb1d412320aa8f305312a0d5 --- /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 ) ); + } +} 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 0000000000000000000000000000000000000000..53d6e71c6b33002e9d12212c32e0d6867836e582 --- /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 ); + } +} diff --git a/compact/components/UT/2024-v00.00/parameters.xml b/compact/components/UT/2024-v00.00/parameters.xml index 975c1670eb3c4f960285be6466bf360b3c9de27e..3c30e6ccb341be203ac604b1598677059f7e41bf 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 647c67debad1f15ac10670c97112ad4572ce9145..3c30e6ccb341be203ac604b1598677059f7e41bf 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 7d976324fd96db7a689a39a07745ce2c1162f424..ec30eddb29d86aaccd0e0cd0fd42e3521c881152 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 0250992e50b438fbc70b5db2009385d9ca12ff52..7ee808d202cb43b2ad6bd41dff4b0bdb3980d28e 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 7d976324fd96db7a689a39a07745ce2c1162f424..ec30eddb29d86aaccd0e0cd0fd42e3521c881152 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 8c6fc0c82ef5b70dfe3917990cbd566be5b5df00..9deec77a818bb1fb730a90c5dd1d7674c077a67f 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 0000000000000000000000000000000000000000..c7f1d816b7e2f6ae9c9968a7997003892ca85c97 --- /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