HGCal validation histograms refactor by fdoljanin · Pull Request #51565 · cms-sw/cmssw

fdoljanin · 2026-07-27T01:29:54Z

This PR simplifies the HGCal trackster validation code. Histogram booking and filling are now defined in one place using HistogramBuilder instead of using many separate vectors and repeated blocks of code.

It keeps the existing 1D plots.

PR validation:

Successfully compiled with scram b in CMSSW_20_1_0_pre1.
Successfully generated workflow 34554.0 with ExtendedRun4D121.

Backport:

Not a backport; no backport is currently planned.

cmsbuild · 2026-07-27T01:30:21Z

cms-bot internal usage

cmsbuild · 2026-07-27T01:32:36Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51565/50324

There are other open Pull requests which might conflict with changes you have proposed:
- File Validation/HGCalValidation/interface/HGVHistoProducerAlgo.h modified in PR(s): Add directories for TICL-EGM code development #48319, Update TICL & HGCal simulation truth validation to use new simulation truth collections, and simplify validation to use only SimCluster dataformat #51047, TICLGeom: a portable SoA geometry #51563
- File Validation/HGCalValidation/plugins/HGCalValidator.cc modified in PR(s): Particle Flow cluster validation in ECAL #50292, Update TICL & HGCal simulation truth validation to use new simulation truth collections, and simplify validation to use only SimCluster dataformat #51047, TICLGeom: a portable SoA geometry #51563
- File Validation/HGCalValidation/src/HGVHistoProducerAlgo.cc modified in PR(s): Update TICL & HGCal simulation truth validation to use new simulation truth collections, and simplify validation to use only SimCluster dataformat #51047, TICLGeom: a portable SoA geometry #51563

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51565/50324/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51565/50324/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

felicepantaleo · 2026-07-27T07:56:22Z

can you point me to a link to a generated plot folder?

fdoljanin · 2026-07-27T11:00:47Z

can you point me to a link to a generated plot folder?

https://fdoljani.web.cern.ch/2D/offline/offline_ticlTrackstersCLUE3DHigh/Efficiencies_byLCs/ here are some examples

cmsbuild · 2026-07-27T11:05:21Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51565/50326

There are other open Pull requests which might conflict with changes you have proposed:
- File Validation/HGCalValidation/interface/HGVHistoProducerAlgo.h modified in PR(s): Add directories for TICL-EGM code development #48319, Update TICL & HGCal simulation truth validation to use new simulation truth collections, and simplify validation to use only SimCluster dataformat #51047, TICLGeom: a portable SoA geometry #51563
- File Validation/HGCalValidation/plugins/HGCalValidator.cc modified in PR(s): Particle Flow cluster validation in ECAL #50292, Update TICL & HGCal simulation truth validation to use new simulation truth collections, and simplify validation to use only SimCluster dataformat #51047, TICLGeom: a portable SoA geometry #51563
- File Validation/HGCalValidation/src/HGVHistoProducerAlgo.cc modified in PR(s): Update TICL & HGCal simulation truth validation to use new simulation truth collections, and simplify validation to use only SimCluster dataformat #51047, TICLGeom: a portable SoA geometry #51563

cmsbuild · 2026-07-27T11:05:46Z

Pull request #51565 was updated.

fdoljanin · 2026-07-27T11:06:27Z

+                name=cms.untracked.string(f"purity_{variable}_{association_suffix}"),
+                title=cms.untracked.string(f"Trackster Purity vs {variable_title}"),
+                numerator=cms.untracked.string(f"Num_{association}_{histogram_variable}"),
+                denominator=cms.untracked.string(f"Denom_{association}_{histogram_variable}"),


I kept this "complicated" way of resolving ratio histograms to keep backwards compatibility, just rewrote it a bit but contract with step3 is the same as before.

cmsbuild · 2026-07-27T11:07:30Z

A new Pull Request was created by @fdoljanin for master.

It involves the following packages:

Validation/HGCalValidation (dqm)

@cmsbuild, @ctarricone, @gabrielmscampos, @rseidita can you please review it and eventually sign? Thanks.
@apsallid, @bsunanda, @cseez, @denizsun, @hatakeyamak, @lecriste, @lgray, @pfs, @rovere, @salimcerci, @sethzenz, @vandreev11 this is something you requested to watch as well.
@ftenchini, @mandrenguyen, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

felicepantaleo · 2026-07-27T11:24:18Z

assign hgcal-dpg

cmsbuild · 2026-07-27T11:24:41Z

New categories assigned: hgcal-dpg

@cseez,@felicepantaleo,@pfs,@rovere you have been requested to review this Pull request/Issue and eventually sign? Thanks

fdoljanin · 2026-07-27T12:28:30Z

type ngt

felicepantaleo · 2026-07-27T12:41:58Z

aren't you mixing in the same PR, 2D histograms and architectural changes?

fdoljanin · 2026-07-28T10:40:59Z

aren't you mixing in the same PR, 2D histograms and architectural changes?

I had to make architectural changes in order to have efficient 2D handling - otherwise number of HGVHistoProducerAlgo would scale as number_of_metrics * number_of_variable_combos.

If you want, I can split them in 2 PRs - keep architecture here and move 2D plots to another!

felicepantaleo · 2026-07-28T12:14:53Z

if it is not too much trouble, split it in two, because I fear the architectural change is introducing some thread-safety issue.

cmsbuild · 2026-08-05T15:51:37Z

Pull request #51565 was updated. @cmsbuild, @cseez, @ctarricone, @felicepantaleo, @gabrielmscampos, @pfs, @rovere, @rseidita can you please check and sign again.

mmusich · 2026-08-06T06:05:16Z

Can the amount of commits be squashed to a minimum, to remove the various "fix" and "format" ?

fdoljanin · 2026-08-06T19:25:34Z

Can the amount of commits be squashed to a minimum, to remove the various "fix" and "format" ?

Done. Just out of curiosity, is there a reason we rewrite the branch history instead of using GitHub’s “Squash and merge” option when merging the PR?

cmsbuild · 2026-08-06T19:25:46Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51565/50489

There are other open Pull requests which might conflict with changes you have proposed:
- File Validation/HGCalValidation/interface/HGVHistoProducerAlgo.h modified in PR(s): Add directories for TICL-EGM code development #48319, Update TICL & HGCal simulation truth validation to use new simulation truth collections, and simplify validation to use only SimCluster dataformat #51047
- File Validation/HGCalValidation/plugins/HGCalValidator.cc modified in PR(s): Particle Flow cluster validation in ECAL #50292, Update TICL & HGCal simulation truth validation to use new simulation truth collections, and simplify validation to use only SimCluster dataformat #51047
- File Validation/HGCalValidation/src/HGVHistoProducerAlgo.cc modified in PR(s): Update TICL & HGCal simulation truth validation to use new simulation truth collections, and simplify validation to use only SimCluster dataformat #51047

cmsbuild · 2026-08-06T19:25:55Z

Pull request #51565 was updated. @cmsbuild, @cseez, @ctarricone, @felicepantaleo, @gabrielmscampos, @pfs, @rovere, @rseidita can you please check and sign again.

mmusich · 2026-08-06T19:39:45Z

@cmsbuild, please test

cmsbuild · 2026-08-06T21:28:39Z

+1

Size: This PR adds an extra 56KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9eda24/55141/summary.html
COMMIT: e1e4f52
CMSSW: CMSSW_20_1_X_2026-08-06-1200/el9_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/51565/55141/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 1 lines from the logs
ROOTFileChecks: Some differences in event products or their sizes found
Reco comparison results: 1 differences found in the comparisons
DQMHistoTests: Total files compared: 45
DQMHistoTests: Total histograms compared: 3589140
DQMHistoTests: Total failures: 648
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3588474
DQMHistoTests: Total skipped: 18
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 44 files compared)
Checked 195 log files, 163 edm output root files, 45 DQM output files
TriggerResults: found differences in 1 / 44 workflows

gabrielmscampos · 2026-08-10T13:13:50Z

+dqm

cmsbuild · 2026-09-09T01:46:52Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51565/50872

ERROR: Build errors found during clang-tidy run.

 2989 |     const SimClusterToCaloParticleMap& scToCpMap) const {
      |           ^
Suppressed 1227 warnings (1223 in non-user code, 4 NOLINT).
--
src/Validation/HGCalValidation/interface/HGVHistoProducerAlgo.h:220:7: error: no template named 'AssociationMap' in namespace 'ticl'; did you mean 'edm::AssociationMap'? [clang-diagnostic-error]
  220 |       ticl::AssociationMap<ticl::oneToOneMapWithFraction, std::vector<SimCluster>, std::vector<CaloParticle>>;
      |       ^~~~~~~~~~~~~~~~~~~~
      |       edm::AssociationMap
--
src/Validation/HGCalValidation/interface/HGVHistoProducerAlgo.h:220:13: error: too many template arguments for class template 'AssociationMap' [clang-diagnostic-error]
  220 |       ticl::AssociationMap<ticl::oneToOneMapWithFraction, std::vector<SimCluster>, std::vector<CaloParticle>>;
      |             ^                                             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02958/el9_amd64_gcc14/cms/cmssw-patch/CMSSW_20_1_X_2026-09-08-2300/src/DataFormats/Common/interface/AssociationMap.h:48:9: note: template is declared here
--
src/Validation/HGCalValidation/interface/HGVHistoProducerAlgo.h:293:45: error: unknown type name 'SimClusterToCaloParticleMap' [clang-diagnostic-error]
  293 |                                       const SimClusterToCaloParticleMap& scToCpMap,
      |                                             ^
src/Validation/HGCalValidation/interface/HGVHistoProducerAlgo.h:365:36: error: unknown type name 'SimClusterToCaloParticleMap' [clang-diagnostic-error]
  365 |                              const SimClusterToCaloParticleMap& scToCpMap) const;
      |                                    ^
Suppressed 1582 warnings (1578 in non-user code, 4 NOLINT).
--
gmake: *** [config/SCRAM/GMake/Makefile.coderules:129: code-checks] Error 2
gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 2

cmsbuild added this to the CMSSW_20_1_X milestone Jul 27, 2026

fdoljanin changed the title ~~2 d hgcal histograms~~ 2D HGCal histograms Jul 27, 2026

cmsbuild added dqm-pending pending-signatures tests-pending orp-pending code-checks-pending labels Jul 27, 2026

cmsbuild added code-checks-rejected and removed code-checks-pending labels Jul 27, 2026

cmsbuild added code-checks-pending and removed code-checks-rejected labels Jul 27, 2026

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 27, 2026

fdoljanin commented Jul 27, 2026

View reviewed changes

fdoljanin marked this pull request as ready for review July 27, 2026 11:06

cmsbuild added the hgcal-dpg-pending label Jul 27, 2026

cmsbuild added the ngt label Jul 27, 2026

HGCal validation histograms refactor

e1e4f52

fdoljanin force-pushed the 2D_hgcal_histograms branch from 0dc0e03 to e1e4f52 Compare August 6, 2026 19:23

cmsbuild added code-checks-pending and removed code-checks-approved labels Aug 6, 2026

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 6, 2026

cmsbuild added tests-started and removed tests-pending labels Aug 6, 2026

cmsbuild added tests-approved and removed tests-started labels Aug 6, 2026

This was referenced Aug 7, 2026

Rename SimDataformats TICL associator as TICLAssociationMap #51649

Merged

Revert PR #51563 #51656

Closed

cmsbuild added dqm-approved and removed dqm-pending labels Aug 10, 2026

Merge branch 'master' into 2D_hgcal_histograms

ffa2215

cmsbuild added tests-pending code-checks-pending and removed tests-approved code-checks-approved labels Sep 9, 2026

cmsbuild added code-checks-rejected and removed code-checks-pending labels Sep 9, 2026

Uh oh!

Conversation

fdoljanin commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmsbuild commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmsbuild commented Jul 27, 2026

Uh oh!

felicepantaleo commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fdoljanin commented Jul 27, 2026

Uh oh!

cmsbuild commented Jul 27, 2026

Uh oh!

cmsbuild commented Jul 27, 2026

Uh oh!

fdoljanin Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

cmsbuild commented Jul 27, 2026

Uh oh!

felicepantaleo commented Jul 27, 2026

Uh oh!

cmsbuild commented Jul 27, 2026

Uh oh!

fdoljanin commented Jul 27, 2026

Uh oh!

felicepantaleo commented Jul 27, 2026

Uh oh!

fdoljanin commented Jul 28, 2026

Uh oh!

felicepantaleo commented Jul 28, 2026

Uh oh!

cmsbuild commented Aug 5, 2026

Uh oh!

mmusich commented Aug 6, 2026

Uh oh!

fdoljanin commented Aug 6, 2026

Uh oh!

cmsbuild commented Aug 6, 2026

Uh oh!

cmsbuild commented Aug 6, 2026

Uh oh!

mmusich commented Aug 6, 2026

Uh oh!

cmsbuild commented Aug 6, 2026

Comparison Summary

Uh oh!

gabrielmscampos commented Aug 10, 2026

Uh oh!

cmsbuild commented Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fdoljanin commented Jul 27, 2026 •

edited

Loading

cmsbuild commented Jul 27, 2026 •

edited

Loading

felicepantaleo commented Jul 27, 2026 •

edited

Loading