From 3b0ea0e9e33bbb016e2258b24cfef6e2ae91a48f Mon Sep 17 00:00:00 2001 From: Laurent Aphecetche Date: Thu, 11 Feb 2021 16:39:38 +0100 Subject: [PATCH] :bug: [MCH] fix bug in Segmentation::findPadPairByPosition In the case where this method returns false (at least one cathode plane has no pad at this position), the pass-by-reference b and nb indices were not (always) meaningful, preventing to deduce which side (bending, non bending or both) was actually missing. Also rationalize the meaning of isValid(padindex) for both Cathode and Segmentation levels : a padindex is valid if it's in the range 0..npads-1 where npads is the number of pads for either the cathode (for Cathode object) or both cathodes (for Segmentation object). --- .../Impl3/src/CathodeSegmentationCImpl3.cxx | 2 +- .../Impl3/src/CathodeSegmentationImpl3.cxx | 5 ++ .../Impl3/src/CathodeSegmentationImpl3.h | 2 + .../Impl4/src/CathodeSegmentationCImpl4.cxx | 2 +- .../Impl4/src/CathodeSegmentationImpl4.cxx | 5 ++ .../Impl4/src/CathodeSegmentationImpl4.h | 2 + .../MCHMappingInterface/Segmentation.inl | 9 ++- .../Mapping/test/src/CathodeSegmentation.cxx | 15 ++++ .../MCH/Mapping/test/src/Segmentation.cxx | 69 ++++++++++++++++++- 9 files changed, 101 insertions(+), 10 deletions(-) diff --git a/Detectors/MUON/MCH/Mapping/Impl3/src/CathodeSegmentationCImpl3.cxx b/Detectors/MUON/MCH/Mapping/Impl3/src/CathodeSegmentationCImpl3.cxx index b52d0dfb8b25d..e26cda5e212cf 100644 --- a/Detectors/MUON/MCH/Mapping/Impl3/src/CathodeSegmentationCImpl3.cxx +++ b/Detectors/MUON/MCH/Mapping/Impl3/src/CathodeSegmentationCImpl3.cxx @@ -89,7 +89,7 @@ void mchCathodeSegmentationForOneDetectionElementOfEachSegmentationType(MchDetec O2MCHMAPPINGIMPL3_EXPORT int mchCathodeSegmentationIsPadValid(MchCathodeSegmentationHandle segHandle, int catPadIndex) { - return catPadIndex != segHandle->impl->InvalidCatPadIndex; + return segHandle->impl->isValid(catPadIndex); } O2MCHMAPPINGIMPL3_EXPORT diff --git a/Detectors/MUON/MCH/Mapping/Impl3/src/CathodeSegmentationImpl3.cxx b/Detectors/MUON/MCH/Mapping/Impl3/src/CathodeSegmentationImpl3.cxx index b7fd6d8548dd9..0c5f7243d4407 100644 --- a/Detectors/MUON/MCH/Mapping/Impl3/src/CathodeSegmentationImpl3.cxx +++ b/Detectors/MUON/MCH/Mapping/Impl3/src/CathodeSegmentationImpl3.cxx @@ -160,6 +160,11 @@ std::vector CathodeSegmentation::getNeighbouringCatPadIndexs(int catPadInde return pads; } +bool CathodeSegmentation::isValid(int catPadIndex) const +{ + return catPadIndex >= 0 && catPadIndex < static_cast(mCatPadIndex2PadGroupIndex.size()); +} + double CathodeSegmentation::squaredDistance(int catPadIndex, double x, double y) const { double px = padPositionX(catPadIndex) - x; diff --git a/Detectors/MUON/MCH/Mapping/Impl3/src/CathodeSegmentationImpl3.h b/Detectors/MUON/MCH/Mapping/Impl3/src/CathodeSegmentationImpl3.h index e334affb63284..d79726938a72d 100644 --- a/Detectors/MUON/MCH/Mapping/Impl3/src/CathodeSegmentationImpl3.h +++ b/Detectors/MUON/MCH/Mapping/Impl3/src/CathodeSegmentationImpl3.h @@ -78,6 +78,8 @@ class CathodeSegmentation int padDualSampaChannel(int catPadIndex) const; + bool isValid(int catPadIndex) const; + private: int dualSampaIndex(int dualSampaId) const; diff --git a/Detectors/MUON/MCH/Mapping/Impl4/src/CathodeSegmentationCImpl4.cxx b/Detectors/MUON/MCH/Mapping/Impl4/src/CathodeSegmentationCImpl4.cxx index 89a3fad70c517..b440e3345acfb 100644 --- a/Detectors/MUON/MCH/Mapping/Impl4/src/CathodeSegmentationCImpl4.cxx +++ b/Detectors/MUON/MCH/Mapping/Impl4/src/CathodeSegmentationCImpl4.cxx @@ -107,7 +107,7 @@ O2MCHMAPPINGIMPL4_EXPORT void O2MCHMAPPINGIMPL4_EXPORT int mchCathodeSegmentationIsPadValid( MchCathodeSegmentationHandle segHandle, int catPadIndex) { - return catPadIndex != segHandle->impl->InvalidCatPadIndex; + return segHandle->impl->isValid(catPadIndex); } O2MCHMAPPINGIMPL4_EXPORT void mchCathodeSegmentationForEachPadInDualSampa( diff --git a/Detectors/MUON/MCH/Mapping/Impl4/src/CathodeSegmentationImpl4.cxx b/Detectors/MUON/MCH/Mapping/Impl4/src/CathodeSegmentationImpl4.cxx index 6f3322f7c1953..71a3c818643c7 100644 --- a/Detectors/MUON/MCH/Mapping/Impl4/src/CathodeSegmentationImpl4.cxx +++ b/Detectors/MUON/MCH/Mapping/Impl4/src/CathodeSegmentationImpl4.cxx @@ -172,6 +172,11 @@ std::vector CathodeSegmentation::getNeighbouringCatPadIndexs( return pads; } +bool CathodeSegmentation::isValid(int catPadIndex) const +{ + return catPadIndex >= 0 && catPadIndex < static_cast(mCatPadIndex2PadGroupIndex.size()); +} + double CathodeSegmentation::squaredDistance(int catPadIndex, double x, double y) const { diff --git a/Detectors/MUON/MCH/Mapping/Impl4/src/CathodeSegmentationImpl4.h b/Detectors/MUON/MCH/Mapping/Impl4/src/CathodeSegmentationImpl4.h index 58770c4b9f03f..909ee640b39ec 100644 --- a/Detectors/MUON/MCH/Mapping/Impl4/src/CathodeSegmentationImpl4.h +++ b/Detectors/MUON/MCH/Mapping/Impl4/src/CathodeSegmentationImpl4.h @@ -88,6 +88,8 @@ class CathodeSegmentation int padDualSampaChannel(int catPadIndex) const; + bool isValid(int catPadIndex) const; + private: int dualSampaIndex(int dualSampaId) const; diff --git a/Detectors/MUON/MCH/Mapping/Interface/include/MCHMappingInterface/Segmentation.inl b/Detectors/MUON/MCH/Mapping/Interface/include/MCHMappingInterface/Segmentation.inl index 2dd0d5685bc84..1de56e0329c89 100644 --- a/Detectors/MUON/MCH/Mapping/Interface/include/MCHMappingInterface/Segmentation.inl +++ b/Detectors/MUON/MCH/Mapping/Interface/include/MCHMappingInterface/Segmentation.inl @@ -65,10 +65,7 @@ inline int Segmentation::findPadByFEE(int dualSampaId, int dualSampaChannel) con inline bool Segmentation::isValid(int dePadIndex) const { - if (dePadIndex < mPadIndexOffset) { - return mBending.isValid(dePadIndex); - } - return mNonBending.isValid(dePadIndex - mPadIndexOffset); + return dePadIndex >= 0 && dePadIndex < nofPads(); } inline int Segmentation::padC2DE(int catPadIndex, bool isBending) const @@ -99,7 +96,9 @@ inline bool Segmentation::findPadPairByPosition(double x, double y, int& b, int& b = mBending.findPadByPosition(x, y); nb = mNonBending.findPadByPosition(x, y); if (!mBending.isValid(b)) { - nb = padC2DE(nb, false); + if (mNonBending.isValid(nb)) { + nb = padC2DE(nb, false); + } return false; } if (!mNonBending.isValid(nb)) { diff --git a/Detectors/MUON/MCH/Mapping/test/src/CathodeSegmentation.cxx b/Detectors/MUON/MCH/Mapping/test/src/CathodeSegmentation.cxx index c211e64b93f29..ffdf2bfb19b60 100644 --- a/Detectors/MUON/MCH/Mapping/test/src/CathodeSegmentation.cxx +++ b/Detectors/MUON/MCH/Mapping/test/src/CathodeSegmentation.cxx @@ -29,6 +29,7 @@ #include #include #include "TestParameters.h" +#include using namespace o2::mch::mapping; namespace bdata = boost::unit_test::data; @@ -299,6 +300,20 @@ BOOST_AUTO_TEST_CASE(ThrowsIfDualSampaChannelIsNotBetween0And63) BOOST_CHECK_THROW(seg.findPadByFEE(102, 64), std::out_of_range); } +BOOST_AUTO_TEST_CASE(ReturnsFalseIfCatPadIdIsOutOfRange) +{ + forOneDetectionElementOfEachSegmentationType([](int detElemId) { + for (auto plane : {true, false}) { + CathodeSegmentation seg{detElemId, plane}; + BOOST_TEST_INFO_SCOPE(fmt::format("DeId {} Bending {}", detElemId, plane)); + BOOST_CHECK_EQUAL(seg.isValid(0), true); + BOOST_CHECK_EQUAL(seg.isValid(seg.nofPads() - 1), true); + BOOST_CHECK_EQUAL(seg.isValid(-1), false); + BOOST_CHECK_EQUAL(seg.isValid(seg.nofPads()), false); + } + }); +} + BOOST_AUTO_TEST_CASE(ReturnsTrueIfPadIsConnected) { BOOST_CHECK_EQUAL(seg.isValid(seg.findPadByFEE(102, 3)), true); diff --git a/Detectors/MUON/MCH/Mapping/test/src/Segmentation.cxx b/Detectors/MUON/MCH/Mapping/test/src/Segmentation.cxx index b56742ae5da8a..a6fead4d5e294 100644 --- a/Detectors/MUON/MCH/Mapping/test/src/Segmentation.cxx +++ b/Detectors/MUON/MCH/Mapping/test/src/Segmentation.cxx @@ -24,6 +24,7 @@ #include #include #include "TestParameters.h" +#include using namespace o2::mch::mapping; namespace bdata = boost::unit_test::data; @@ -186,7 +187,7 @@ BOOST_AUTO_TEST_CASE(CheckPadOffsetsAfterCopy) }); } -struct SEG { +struct SEG100 { Segmentation seg{100}; }; @@ -210,9 +211,60 @@ BOOST_AUTO_TEST_CASE(TestForEachPadAndPadIndexRange) }); } -// All the remaining tests of this file are using seg (DE100). +BOOST_AUTO_TEST_CASE(CheckOnePadPositionPresentOnOnlyBendingPlaneDE600) +{ + Segmentation de600(600); + double x = 54.34; + double y = -12.40; + int b, nb; + bool ok = de600.findPadPairByPosition(x, y, b, nb); + TestParameters params; + int testChannel = 12; + if (params.isSegmentationRun3) { + testChannel = 44; + } + BOOST_CHECK_EQUAL(ok, false); + int p = de600.findPadByFEE(307, testChannel); + BOOST_CHECK_EQUAL(p, b); + BOOST_CHECK_EQUAL(de600.isValid(b), true); + BOOST_CHECK_EQUAL(de600.isValid(nb), false); +} + +BOOST_AUTO_TEST_CASE(CheckOnePadPositionPresentOnOnlyBendingPlaneDE825) +{ + Segmentation de825(825); + double x = 110.09; + double y = -1.25; + int b, nb; + bool ok = de825.findPadPairByPosition(x, y, b, nb); + TestParameters params; + int testChannel{55}; + if (params.isSegmentationRun3) { + testChannel = 23; + } + BOOST_CHECK_EQUAL(ok, false); + int p = de825.findPadByFEE(1333, testChannel); + BOOST_CHECK_EQUAL(p, nb); + BOOST_CHECK_EQUAL(de825.isValid(b), false); + BOOST_CHECK_EQUAL(de825.isValid(nb), true); +} + +BOOST_AUTO_TEST_CASE(CheckOnePadPositionTotallyAbsentDE604) +{ + Segmentation de604(604); + double x = -19.25; + double y = 20.04; + int b, nb; + bool ok = de604.findPadPairByPosition(x, y, b, nb); + BOOST_CHECK_EQUAL(ok, false); + BOOST_CHECK_EQUAL(de604.isValid(nb), false); + BOOST_CHECK_EQUAL(de604.isValid(b), false); +} -BOOST_FIXTURE_TEST_SUITE(DE100, SEG) +// All the remaining tests of this file are using specific +// detection elements DE100 + +BOOST_FIXTURE_TEST_SUITE(DE100, SEG100) BOOST_TEST_DECORATOR(*boost::unit_test::tolerance(1E-3)) BOOST_AUTO_TEST_CASE(CheckOnePosition) @@ -373,6 +425,17 @@ BOOST_AUTO_TEST_CASE(ReturnsFalseIfPadIsNotConnected) BOOST_CHECK_EQUAL(seg.isValid(seg.findPadByFEE(214, testChannel)), false); } +BOOST_AUTO_TEST_CASE(ReturnsFalseIfCatPadIdIsOutOfRange) +{ + forOneDetectionElementOfEachSegmentationType([](int detElemId) { + Segmentation seg{detElemId}; + BOOST_TEST_INFO_SCOPE(fmt::format("DeId {}", detElemId)); + BOOST_CHECK_EQUAL(seg.isValid(0), true); + BOOST_CHECK_EQUAL(seg.isValid(seg.nofPads() - 1), true); + BOOST_CHECK_EQUAL(seg.isValid(-1), false); + BOOST_CHECK_EQUAL(seg.isValid(seg.nofPads()), false); + }); +} BOOST_AUTO_TEST_CASE(HasPadByPosition) { int b, nb;