MCH: added plots per cru link #2592

aferrero2707 · 2025-08-05T16:36:33Z

The PR introduces the code needed to plot relevant quantities (rates, efficiencies, fractions of good channels, etc...) averaged over each SOLAR board (i.e. CRU link). The final goal is to detect links that are bad in either of the monitored quantities, and store a list of bad links in the CCDB to allow automated re-configuration during the data taking.

The new code adds the following plots:

qc/MCH/MO/Decoding/QualityFlagPerSolar
qc/MCH/MO/Decoding/DecodingErrors/GoodBoardsFractionPerSolar
qc/MCH/MO/Decoding/DecodingErrors/LastCycle/GoodBoardsFractionPerSolar
qc/MCH/MO/Decoding/SyncErrors/SyncedBoardsFractionPerSolar
qc/MCH/MO/Decoding/SyncErrors/LastCycle/SyncedBoardsFractionPerSolar
qc/MCH/MO/Digits/QualityFlagPerSolar
qc/MCH/MO/Digits/Rates/GoodChannelsFractionPerSolar
qc/MCH/MO/Digits/Rates/LastCycle/GoodChannelsFractionPerSolar
qc/MCH/MO/Digits/Rates/MeanRatePerSolar
qc/MCH/MO/Digits/Rates/LastCycle/MeanRatePerSolar
qc/MCH/MO/Digits/RatesSignal/GoodChannelsFractionPerSolar
qc/MCH/MO/Digits/RatesSignal/LastCycle/GoodChannelsFractionPerSolar
qc/MCH/MO/Digits/RatesSignal/MeanRatePerSolar
qc/MCH/MO/Digits/RatesSignal/LastCycle/MeanRatePerSolar
qc/MCH/MO/Preclusters/QualityFlagPerSolar
qc/MCH/MO/Preclusters/Efficiency/MeanEfficiencyPerSolar
qc/MCH/MO/Preclusters/Efficiency/LastCycle/MeanEfficiencyPerSolar

It also adds some comparisons with reference plots (using the feature introduced in #2578):

qc/MCH/MO/Decoding/RefComp/DecodingErrors/GoodBoardsFractionPerSolar
qc/MCH/MO/Decoding/RefComp/DecodingErrors/GoodBoardsFractionPerDE
qc/MCH/MO/Digits/RefComp/RatesSignal/GoodChannelsFraction
qc/MCH/MO/Digits/RefComp/RatesSignal/GoodChannelsFractionPerSolar
qc/MCH/MO/Digits/RefComp/RatesSignal/MeanRate
qc/MCH/MO/Digits/RefComp/RatesSignal/MeanRatePerSolar
qc/MCH/MO/Preclusters/RefComp/Efficiency/MeanEfficiencyB
qc/MCH/MO/Preclusters/RefComp/Efficiency/MeanEfficiencyNB
qc/MCH/MO/Preclusters/RefComp/Efficiency/MeanEfficiencyPerSolar

See the description of the individual commits for details.

There is some code duplication in the tasks, but I prefer to address it in a future PR, because some refactoring of the MCH code will be anyhow needed at this point.

Added computation of average quantities over SOLAR links, similar to what is already implemented for the averaging over Detection Elements

… decorations

The changes are needed to sync with the updated helper code

The commit adds plots with quantities averaged over SOLAR links, similar to the existing ones as function of Detection Elements. It also adds comparisons of DE and SOLAR plots with reference ones, using the code from AliceO2Group#2578

The code that checks the SOLAR-based plots is similar to that for the DE-based ones. The checkers also fill a summary plot with the overall quality of each SOLAR link, which are then aggregared by a dedicated task.

The task combines the summary qualities for each DE and SOLAR link, and uploads a CCDB object with the list of bad DE and SOLAR IDs. The CCDB objects will be used to automatically reconfigure the bad links during the data taking.

aferrero2707 · 2025-08-06T10:22:37Z

@Barthelemy @knopers8 @justonedev1 sorry for the rather large PR, it actually contains similar code modifications in three MCH tasks and corresponding checkers.

The changes introduce the possibility to propagate the results of the QC checks back to the DCS (via CCDB objects), such that we can then automate the reconfiguration of bad CRU links during the data taking.

knopers8

Thank you! I did not manage to spot any real issue. I left some pedantic suggestions for your consideration, but it can be merged as it is if needed.

Modules/MUON/MCH/include/MCH/DecodingCheck.h

knopers8 · 2025-08-07T07:40:52Z

Modules/MUON/MCH/include/MCH/PreclustersPostProcessing.h

  static std::string clusterChargeSourceName() { return "clcharge"; }
  static std::string clusterSizeSourceName() { return "clsize"; }

+  TH1* getHistogram(std::string plotName);


I don't think this might become a bottleneck here, but in principle it's better to avoid copies.

Suggested change

TH1* getHistogram(std::string plotName);

TH1* getHistogram(const std::string& plotName); // or std::string_view

I agree, I will use the string_view

knopers8 · 2025-08-07T07:41:55Z

Modules/MUON/MCH/include/MCH/QualityAggregatorTask.h

+///
+/// \file    QualityAggregatorTask.h
+/// \author  Andrea Ferrero andrea.ferrero@cern.ch
+/// \brief   Post-processing of the MCH pre-clusters


This line seems to be a copy leftover.

knopers8 · 2025-08-07T07:42:00Z

Modules/MUON/MCH/include/MCH/QualityAggregatorTask.h

+namespace o2::quality_control_modules::muonchambers
+{
+
+/// \brief  A post-processing task which processes and trends MCH pre-clusters and produces plots.


This line seems to be a copy leftover.

knopers8 · 2025-08-07T08:08:16Z

Modules/MUON/MCH/src/Helpers.cxx

+      if (deId < 0) {
+        deId = lMax + (nDEhc - indexInHalfChamber);
+      }
+      /*if (indexInHalfChamber > nDEqc) {


Consider removing commented out code unless it is needed for soon development. The debug logs modified to use e.g. ILOG(Trace, Devel).

Yes, indeed thanks for spotting it

knopers8 · 2025-08-07T08:19:44Z

Modules/MUON/MCH/src/Helpers.cxx

+  try {
+    return m.at(solarId);
+  } catch (const std::exception&) {
+    ILOG(Error, Support) << "Invalid Solar Id: " << solarId;


Suggested change

ILOG(Error, Support) << "Invalid Solar Id: " << solarId;

ILOG(Error, Support) << "Invalid Solar Id: " << solarId << ENDM;

missing ENDM, same below

Good catch.

knopers8 · 2025-08-07T08:20:53Z

Modules/MUON/MCH/src/Helpers.cxx

+/*
+constexpr int getNumSolar()
+{
+  //static std::vector<uint32_t> v = buildSolarIndexToSolarIdMap();
+  //return v.size();
+  return 624;
+}
+*/


still needed?

No, will remove it

knopers8 · 2025-08-07T09:19:13Z

Modules/MUON/MCH/src/QualityAggregatorTask.cxx

+    return { nullptr, false };
+  }
+  // retrieve QO from CCDB
+  auto qo = qcdb.retrieveMO(path, name, objectTimestamp, trigger.activity);


Out of curiosity, why not ask the method to take care of retrieving the latest object? As far as I understand, you wouldn't need most of the code above in such case.

Suggested change

auto qo = qcdb.retrieveMO(path, name, objectTimestamp, trigger.activity);

auto qo = qcdb.retrieveMO(path, name, Timestamp::Latest, trigger.activity);

You are probably right, but I need to think a bit more about it. Since it is not really a bug in the code, but a possible simplification, I propose to leave it as it is for the moment. In any case I am planning some code refactoring in the YETS.

Sure, no problem. I was just curious if we miss a feature.

aferrero2707 · 2025-08-07T13:51:26Z

@knopers8 thanks for the review! I can push the proposed changes this evening, such that you could then merge tomorrow. Would it be fine for you?

knopers8 · 2025-08-07T14:17:43Z

@knopers8 thanks for the review! I can push the proposed changes this evening, such that you could then merge tomorrow. Would it be fine for you?

Yes, no problem. But the QC release will probably happen next week anyway, unless you need a patch with that.

aferrero2707 · 2025-08-07T14:19:35Z

Thanks! No patch needed, I just want to make sure this one is included in next week's upgrade, since it will bring some simplification to our on-call operations.

* [MCH] added averaging over SOLAR links Added computation of average quantities over SOLAR links, similar to what is already implemented for the averaging over Detection Elements * [MCH] added helper functions for handling SOLAR indexes and histogram decorations * [MCH] updated Pedestals Task histogram decorations The changes are needed to sync with the updated helper code * [MCH] added plots as function of SOLAR links The commit adds plots with quantities averaged over SOLAR links, similar to the existing ones as function of Detection Elements. It also adds comparisons of DE and SOLAR plots with reference ones, using the code from #2578 * [MCH] added checkers for SOLAR-based plots The code that checks the SOLAR-based plots is similar to that for the DE-based ones. The checkers also fill a summary plot with the overall quality of each SOLAR link, which are then aggregared by a dedicated task. * [MCH] added aggregator task for DE and SOLAR qualities The task combines the summary qualities for each DE and SOLAR link, and uploads a CCDB object with the list of bad DE and SOLAR IDs. The CCDB objects will be used to automatically reconfigure the bad links during the data taking. * [MCH] fixed clang errors * [MCH] added review suggestions

The missing initializations are related to new parameters introduced in AliceO2Group#2592

…#2601) The missing initializations are related to new parameters introduced in #2592

aferrero2707 added 6 commits August 5, 2025 17:35

[MCH] added averaging over SOLAR links

ed707b7

Added computation of average quantities over SOLAR links, similar to what is already implemented for the averaging over Detection Elements

[MCH] added helper functions for handling SOLAR indexes and histogram…

098c544

… decorations

[MCH] updated Pedestals Task histogram decorations

2d09135

The changes are needed to sync with the updated helper code

[MCH] added plots as function of SOLAR links

b900cf7

The commit adds plots with quantities averaged over SOLAR links, similar to the existing ones as function of Detection Elements. It also adds comparisons of DE and SOLAR plots with reference ones, using the code from AliceO2Group#2578

[MCH] added checkers for SOLAR-based plots

6409144

The code that checks the SOLAR-based plots is similar to that for the DE-based ones. The checkers also fill a summary plot with the overall quality of each SOLAR link, which are then aggregared by a dedicated task.

[MCH] added aggregator task for DE and SOLAR qualities

9ccfc80

The task combines the summary qualities for each DE and SOLAR link, and uploads a CCDB object with the list of bad DE and SOLAR IDs. The CCDB objects will be used to automatically reconfigure the bad links during the data taking.

aferrero2707 requested review from Barthelemy, justonedev1 and knopers8 as code owners August 5, 2025 16:36

[MCH] fixed clang errors

a90f036

knopers8 approved these changes Aug 7, 2025

View reviewed changes

[MCH] added review suggestions

0056180

knopers8 merged commit 6283263 into AliceO2Group:master Aug 8, 2025
6 checks passed

aferrero2707 added a commit to AliceMCH/QualityControl that referenced this pull request Aug 19, 2025

[MCH] added some missing parameter initializations in DecodingChecker

2c76a51

The missing initializations are related to new parameters introduced in AliceO2Group#2592

aferrero2707 mentioned this pull request Aug 19, 2025

MCH: added some missing parameter initializations in DecodingChecker #2601

Merged

Barthelemy pushed a commit that referenced this pull request Aug 20, 2025

[MCH] added some missing parameter initializations in DecodingChecker (…

b371f7c

…#2601) The missing initializations are related to new parameters introduced in #2592

	TH1* getHistogram(std::string plotName);
	TH1* getHistogram(const std::string& plotName); // or std::string_view

	ILOG(Error, Support) << "Invalid Solar Id: " << solarId;
	ILOG(Error, Support) << "Invalid Solar Id: " << solarId << ENDM;

	auto qo = qcdb.retrieveMO(path, name, objectTimestamp, trigger.activity);
	auto qo = qcdb.retrieveMO(path, name, Timestamp::Latest, trigger.activity);

MCH: added plots per cru link #2592

MCH: added plots per cru link #2592

Uh oh!

Conversation

aferrero2707 commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aferrero2707 commented Aug 6, 2025

Uh oh!

knopers8 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aferrero2707 commented Aug 7, 2025

Uh oh!

knopers8 commented Aug 7, 2025

Uh oh!

aferrero2707 commented Aug 7, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

aferrero2707 commented Aug 5, 2025 •

edited

Loading