[PWGDQ] ML response for DQ-analysis selections #12169

JinjooSeo · 2025-07-21T11:26:46Z

Implement ML-based response class (DQMlResponse) for dielectron DQ-analysis selections
Supports both binary and multiclass BDT evaluation using ONNX
Example JSON files included for reference

github-actions · 2025-07-21T11:27:03Z

O2 linter results: ❌ 1308 errors, ⚠️ 0 warnings, 🔕 0 disabled

vkucera · 2025-07-21T13:56:00Z

cppcheck reports bugs. Please fix.

iarsene · 2025-07-21T12:58:14Z

PWGDQ/Core/DQMlResponse.h

+enum class InputFeatures : uint8_t { // refer to DielectronsAll
+  fMass = 0,
+  fPt,
+  fEta,
+  fPhi,
+  fPt1,
+  fITSChi2NCl1,
+  fTPCNClsCR1,
+  fTPCNClsFound1,
+  fTPCChi2NCl1,
+  fDcaXY1,
+  fDcaZ1,
+  fTPCNSigmaEl1,
+  fTPCNSigmaPi1,
+  fTPCNSigmaPr1,
+  fTOFNSigmaEl1,
+  fTOFNSigmaPi1,
+  fTOFNSigmaPr1,
+  fPt2,
+  fITSChi2NCl2,
+  fTPCNClsCR2,
+  fTPCNClsFound2,
+  fTPCChi2NCl2,
+  fDcaXY2,
+  fDcaZ2,
+  fTPCNSigmaEl2,
+  fTPCNSigmaPi2,
+  fTPCNSigmaPr2,
+  fTOFNSigmaEl2,
+  fTOFNSigmaPi2,
+  fTOFNSigmaPr2,
+};


to keep similar convention as in other parts of DQ code, please name the elements of the enum to start with "k", not "f"

iarsene · 2025-07-21T12:58:42Z

PWGDQ/Core/DQMlResponse.h

+static const std::map<InputFeatures, std::string> gFeatureNameMap = {
+  {InputFeatures::fMass, "fMass"},
+  {InputFeatures::fPt, "fPt"},
+  {InputFeatures::fEta, "fEta"},
+  {InputFeatures::fPhi, "fPhi"},
+  {InputFeatures::fPt1, "fPt1"},
+  {InputFeatures::fITSChi2NCl1, "fITSChi2NCl1"},
+  {InputFeatures::fTPCNClsCR1, "fTPCNClsCR1"},
+  {InputFeatures::fTPCNClsFound1, "fTPCNClsFound1"},
+  {InputFeatures::fTPCChi2NCl1, "fTPCChi2NCl1"},
+  {InputFeatures::fDcaXY1, "fDcaXY1"},
+  {InputFeatures::fDcaZ1, "fDcaZ1"},
+  {InputFeatures::fTPCNSigmaEl1, "fTPCNSigmaEl1"},
+  {InputFeatures::fTPCNSigmaPi1, "fTPCNSigmaPi1"},
+  {InputFeatures::fTPCNSigmaPr1, "fTPCNSigmaPr1"},
+  {InputFeatures::fTOFNSigmaEl1, "fTOFNSigmaEl1"},
+  {InputFeatures::fTOFNSigmaPi1, "fTOFNSigmaPi1"},
+  {InputFeatures::fTOFNSigmaPr1, "fTOFNSigmaPr1"},
+  {InputFeatures::fPt2, "fPt2"},
+  {InputFeatures::fITSChi2NCl2, "fITSChi2NCl2"},
+  {InputFeatures::fTPCNClsCR2, "fTPCNClsCR2"},
+  {InputFeatures::fTPCNClsFound2, "fTPCNClsFound2"},
+  {InputFeatures::fTPCChi2NCl2, "fTPCChi2NCl2"},
+  {InputFeatures::fDcaXY2, "fDcaXY2"},
+  {InputFeatures::fDcaZ2, "fDcaZ2"},
+  {InputFeatures::fTPCNSigmaEl2, "fTPCNSigmaEl2"},
+  {InputFeatures::fTPCNSigmaPi2, "fTPCNSigmaPi2"},
+  {InputFeatures::fTPCNSigmaPr2, "fTPCNSigmaPr2"},
+  {InputFeatures::fTOFNSigmaEl2, "fTOFNSigmaEl2"},
+  {InputFeatures::fTOFNSigmaPi2, "fTOFNSigmaPi2"},
+  {InputFeatures::fTOFNSigmaPr2, "fTOFNSigmaPr2"}};


similar comment as above, start the name with k

iarsene · 2025-07-21T13:49:02Z

PWGDQ/Tasks/tableReader_withAssoc.cxx

+  void processBarrelOnlySkimmedBDT(MyEventsVtxCovSelected const& events,
+                                   soa::Join<aod::ReducedTracksAssoc, aod::BarrelTrackCuts, aod::Prefilter> const& barrelAssocs,
+                                   MyBarrelTracksWithCovWithAmbiguities const& barrelTracks)
+  {
+    runSameEventPairing<true, VarManager::kDecayToEE, gkEventFillMapWithCov, gkTrackFillMapWithCov>(events, trackAssocsPerCollision, barrelAssocs, barrelTracks);
+  }
+


This is not needed, as you can use the process function processBarrelOnlySkimmed.

iarsene · 2025-07-21T13:49:17Z

PWGDQ/Tasks/tableReader_withAssoc.cxx

  PROCESS_SWITCH(AnalysisSameEventPairing, processBarrelOnlySkimmedNoCov, "Run barrel only pairing (no covariances), with skimmed tracks and with collision information", false);
  PROCESS_SWITCH(AnalysisSameEventPairing, processBarrelOnlySkimmedNoCovWithMultExtra, "Run barrel only pairing (no covariances), with skimmed tracks, with collision information, with MultsExtra", false);
  PROCESS_SWITCH(AnalysisSameEventPairing, processBarrelOnlyWithQvectorCentrSkimmedNoCov, "Run barrel only pairing (no covariances), with skimmed tracks, with Qvector from central framework", false);
+  PROCESS_SWITCH(AnalysisSameEventPairing, processBarrelOnlySkimmedBDT, "Run electron-electron pairing, with skimmed tracks and BDT selection", false);


This is also not needed

iarsene · 2025-07-21T13:51:48Z

PWGDQ/Core/DQMlResponse.h

+// Fill the map of available input features
+// the key is the feature's name (std::string)
+// the value is the corresponding value in EnumInputFeatures
+#define FILL_MAP(FEATURE)                                  \
+  {                                                        \
+    #FEATURE, static_cast<uint8_t>(InputFeatures::FEATURE) \
+  }


As we discussed, I suggest to not use this macro

iarsene · 2025-07-21T13:57:56Z

PWGDQ/Core/DQMlResponse.h

+      {"fTPCNSigmaPr2", [](auto const&, auto const& t2, auto const&) { return t2.tpcNSigmaPr(); }},
+      {"fTOFNSigmaEl2", [](auto const&, auto const& t2, auto const&) { return t2.tofNSigmaEl(); }},
+      {"fTOFNSigmaPi2", [](auto const&, auto const& t2, auto const&) { return t2.tofNSigmaPi(); }},
+      {"fTOFNSigmaPr2", [](auto const&, auto const& t2, auto const&) { return t2.tofNSigmaPr(); }}};


This is not needed, as you can fill directly these values in the dqInputFeatures vector

iarsene · 2025-07-21T14:00:50Z

PWGDQ/Core/DQMlResponse.h

+      FILL_MAP(fMass),
+      FILL_MAP(fPt),
+      FILL_MAP(fEta),
+      FILL_MAP(fPhi),
+      FILL_MAP(fPt1),
+      FILL_MAP(fITSChi2NCl1),
+      FILL_MAP(fTPCNClsCR1),
+      FILL_MAP(fTPCNClsFound1),
+      FILL_MAP(fTPCChi2NCl1),
+      FILL_MAP(fDcaXY1),
+      FILL_MAP(fDcaZ1),
+      FILL_MAP(fTPCNSigmaEl1),
+      FILL_MAP(fTPCNSigmaPi1),
+      FILL_MAP(fTPCNSigmaPr1),
+      FILL_MAP(fTOFNSigmaEl1),
+      FILL_MAP(fTOFNSigmaPi1),
+      FILL_MAP(fTOFNSigmaPr1),
+      FILL_MAP(fPt2),
+      FILL_MAP(fITSChi2NCl2),
+      FILL_MAP(fTPCNClsCR2),
+      FILL_MAP(fTPCNClsFound2),
+      FILL_MAP(fTPCChi2NCl2),
+      FILL_MAP(fDcaXY2),
+      FILL_MAP(fDcaZ2),
+      FILL_MAP(fTPCNSigmaEl2),
+      FILL_MAP(fTPCNSigmaPi2),
+      FILL_MAP(fTPCNSigmaPr2),
+      FILL_MAP(fTOFNSigmaEl2),
+      FILL_MAP(fTOFNSigmaPi2),
+      FILL_MAP(fTOFNSigmaPr2)};


as commented earlier, do not use the macro FILL_MAP

iarsene · 2025-07-21T14:01:30Z

PWGDQ/Tasks/tableReader.cxx

+  void processDecayToEESkimmedBDT(soa::Filtered<MyEventsSelected>::iterator const& event, soa::Filtered<MyBarrelTracksSelected> const& tracks)
+  {
+    // Reset the fValues array
+    VarManager::ResetValues(0, VarManager::kNVars);
+    VarManager::FillEvent<gkEventFillMap>(event, VarManager::fgValues);
+    runSameEventPairing<false, VarManager::kDecayToEE, gkEventFillMap, gkTrackFillMap>(event, tracks, tracks);
+  }


this process function is not needed

iarsene · 2025-07-21T14:01:40Z

PWGDQ/Tasks/tableReader.cxx

  PROCESS_SWITCH(AnalysisSameEventPairing, processDecayToEEPrefilterSkimmedNoTwoProngFitter, "Run electron-electron pairing, with skimmed tracks and prefilter from AnalysisPrefilterSelection but no two prong fitter", false);
  PROCESS_SWITCH(AnalysisSameEventPairing, processDecayToEESkimmedWithColl, "Run electron-electron pairing, with skimmed tracks and with collision information", false);
  PROCESS_SWITCH(AnalysisSameEventPairing, processDecayToEESkimmedWithCollNoTwoProngFitter, "Run electron-electron pairing, with skimmed tracks and with collision information but no two prong fitter", false);
+  PROCESS_SWITCH(AnalysisSameEventPairing, processDecayToEESkimmedBDT, "Run electron-electron pairing, with skimmed tracks and BDT selection", false);


process switch not needed

…alysis selections. Supports both binary and multiclass BDT evaluation using ONNX Example JSON files included for reference

PWGDQ/Tasks/tableReader.cxx

PWGDQ/Tasks/tableReader_withAssoc.cxx

iarsene · 2025-08-05T09:40:54Z

PWGDQ/Core/VarManager.h

+template <typename T1>
+void VarManager::FillBdtScore(T1 const& bdtScore, float* values)
+{
+  if (!values) {
+    values = fgValues;
+  }
+
+  if (bdtScore.size() == 1) {
+    values[kBdtBackground] = bdtScore[0];
+  } else if (bdtScore.size() == 3) {
+    values[kBdtBackground] = bdtScore[0];
+    values[kBdtPrompt] = bdtScore[1];
+    values[kBdtNonprompt] = bdtScore[2];
+  } else {
+    LOG(warning) << "Unexpected number of BDT outputs: " << bdtScore.size();
+  }
+}
+


Add a small descriptive comment to the function, for improved readability

iarsene · 2025-08-05T09:44:15Z

PWGDQ/Tasks/tableReader.cxx

+  // ML inference
+  Configurable<bool> applyBDT{"applyBDT", false, "Flag to apply ML selections"};
+  Configurable<std::string> fConfigBdtCutsJSON{"fConfigBdtCutsJSON", "", "Additional list of BDT cuts in JSON format"};
+  Configurable<std::vector<std::string>> modelPathsCCDB{"modelPathsCCDB", std::vector<std::string>{"Users/j/jseo/ML/PbPbPsi/default/"}, "Paths of models on CCDB"};
+  Configurable<int64_t> timestampCCDB{"timestampCCDB", -1, "timestamp of the ONNX file for ML model used to query in CCDB"};
+  Configurable<bool> loadModelsFromCCDB{"loadModelsFromCCDB", false, "Flag to enable or disable the loading of models from CCDB"};


I was discussing in the past that at some point we should get rid of the workflows w/o association, because it requires additional maintaining. So, unless you plan to actually use this workflow, I would suggest to not implement the ML response here, but just in the _withAssoc version. If there will be requests for this to be added also here, we can discuss.

iarsene · 2025-08-05T09:46:44Z

PWGDQ/Tasks/tableReader_withAssoc.cxx

+  o2::analysis::DQMlResponse<float> dqMlResponse;
+  std::vector<float> outputMlPsi2ee = {}; // TODO: check this is needed or not


Please use the same naming conventions as the rest of the file (variable name starts with 'f', since it is a class data member)

@iarsene

https://rawgit.com/AliceO2Group/CodingGuidelines/master/naming_formatting.html?showone=Variable_Names#Variable_Names

Class member variables are prefixed with m.
No m prefix for struct members.

These are struct members.

Sorry Vit, we are not ready to start changing all this now. I was reffering to use the same convention as in the file now. The "f" was used for a long time in ALICE, btw. Its difficult to change styles all the time :)

Sure, I understand. Although the f was used in AliPhysics because it was based on ROOT classes. That is not the case for O2 and the O2 conventions has not changed since the beginning of the project in this sense.

Co-authored-by: Jinjoo Seo <jseo@dhcp187-536.laptop-wlc.uni-heidelberg.de>

JinjooSeo requested review from alibuild, iarsene, lucamicheletti93 and mcoquet642 as code owners July 21, 2025 11:26

github-actions bot added the pwgdq label Jul 21, 2025

vkucera marked this pull request as draft July 21, 2025 13:56

iarsene reviewed Jul 21, 2025

View reviewed changes

Jinjoo Seo and others added 6 commits August 4, 2025 15:31

Implement ML-based response class (DQMlResponse) for dielectron DQ-an…

eb2a74d

…alysis selections. Supports both binary and multiclass BDT evaluation using ONNX Example JSON files included for reference

linter error resolve

9d6591d

clang resolve

2549ae2

new PR

03a3844

Address comments

5707bee

clang format

6516a47

JinjooSeo force-pushed the MLframeworkDQ_clean branch from 9b0a2fc to 6516a47 Compare August 4, 2025 13:45

vkucera requested changes Aug 4, 2025

View reviewed changes

PWGDQ/Tasks/tableReader.cxx Outdated Show resolved Hide resolved

PWGDQ/Tasks/tableReader_withAssoc.cxx Outdated Show resolved Hide resolved

vkucera previously approved these changes Aug 4, 2025

View reviewed changes

JinjooSeo dismissed vkucera’s stale review via 615b67c August 4, 2025 15:26

iarsene reviewed Aug 5, 2025

View reviewed changes

apply comments

f90b771

JinjooSeo force-pushed the MLframeworkDQ_clean branch from 615b67c to f90b771 Compare August 5, 2025 14:09

JinjooSeo marked this pull request as ready for review August 5, 2025 14:13

iarsene approved these changes Aug 5, 2025

View reviewed changes

iarsene enabled auto-merge (squash) August 5, 2025 14:43

iarsene merged commit 2c6afcb into AliceO2Group:master Aug 5, 2025
11 of 13 checks passed

jpxrk pushed a commit to jpxrk/O2Physics that referenced this pull request Aug 12, 2025

[PWGDQ] ML response for DQ-analysis selections (AliceO2Group#12169)

110dd42

Co-authored-by: Jinjoo Seo <jseo@dhcp187-536.laptop-wlc.uni-heidelberg.de>

jloemker pushed a commit to jloemker/O2Physics that referenced this pull request Nov 11, 2025

[PWGDQ] ML response for DQ-analysis selections (AliceO2Group#12169)

e016e84

Co-authored-by: Jinjoo Seo <jseo@dhcp187-536.laptop-wlc.uni-heidelberg.de>

alibuild pushed a commit to alibuild/O2Physics that referenced this pull request Dec 5, 2025

[PWGDQ] ML response for DQ-analysis selections (AliceO2Group#12169)

fec8726

Co-authored-by: Jinjoo Seo <jseo@dhcp187-536.laptop-wlc.uni-heidelberg.de>

		o2::analysis::DQMlResponse<float> dqMlResponse;
		std::vector<float> outputMlPsi2ee = {}; // TODO: check this is needed or not

[PWGDQ] ML response for DQ-analysis selections #12169

[PWGDQ] ML response for DQ-analysis selections #12169

Uh oh!

Conversation

JinjooSeo commented Jul 21, 2025

Uh oh!

github-actions bot commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vkucera commented Jul 21, 2025

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!

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!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Jul 21, 2025 •

edited

Loading

iarsene Aug 5, 2025 •

edited

Loading

vkucera Aug 5, 2025 •

edited

Loading