Skip to content

Add ECAL Phase 2 digis and TP digis validation - #51400

Merged
cmsbuild merged 4 commits into
cms-sw:masterfrom
thomreis:ecal-phase2-digi-tp-val
Jul 8, 2026
Merged

Add ECAL Phase 2 digis and TP digis validation#51400
cmsbuild merged 4 commits into
cms-sw:masterfrom
thomreis:ecal-phase2-digi-tp-val

Conversation

@thomreis

@thomreis thomreis commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

PR description:

This PR adds validation histograms for the ECAL Phase 2 digis and TP digis. The validation is activated with the eras phase2_ecal_devel and phase2_ecalTP_devel, respectively.

PR validation:

Tested with the ECAL Phase 2 developments workflows 34434.612, 34634.612, 34634.61299. The newly added histograms appear in the DQM ROOT file and are filled.

@cmsbuild

cmsbuild commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

cms-bot internal usage

@thomreis

thomreis commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

type ecal

@cmsbuild cmsbuild added the ecal label Jul 3, 2026
@cmsbuild

cmsbuild commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51400/50065

  • There are other open Pull requests which might conflict with changes you have proposed:

@cmsbuild

cmsbuild commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

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

It involves the following packages:

  • Validation/Configuration (dqm, simulation)
  • Validation/EcalDigis (dqm)
  • Validation/EcalTriggerPrimitives (l1)

@BenjaminRS, @civanch, @cmsbuild, @ctarricone, @gabrielmscampos, @kpedro88, @mdhildreth, @quinnanm, @rseidita can you please review it and eventually sign? Thanks.
@ReyerBand, @apsallid, @argiro, @denizsun, @missirol, @mmusich, @rchatter, @rovere, @salimcerci, @thomreis, @wang0jin 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

@thomreis

thomreis commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

please test

@cmsbuild

cmsbuild commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

+1

Size: This PR adds an extra 32KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d37982/54493/summary.html
COMMIT: 963781b
CMSSW: CMSSW_20_1_X_2026-07-03-1100/el9_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/51400/54493/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
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 45
  • DQMHistoTests: Total histograms compared: 3547476
  • DQMHistoTests: Total failures: 29
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3547429
  • DQMHistoTests: Total skipped: 18
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 1509.69 KiB( 44 files compared)
  • DQMHistoSizes: changed ( 34434.0,... ): 337.924 KiB EcalDigisV/EcalTriggerPrimitivesTask
  • DQMHistoSizes: changed ( 34434.0,... ): -35.986 KiB EcalDigisV/EcalDigiTask
  • Checked 195 log files, 163 edm output root files, 45 DQM output files
  • TriggerResults: no differences found

@civanch

civanch commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

+simulation

Comment on lines +107 to +164
void EcalDigisValidationPh2::bookHistograms(DQMStore::IBooker& ibooker, edm::Run const&, edm::EventSetup const&) {
Char_t histo[200];

ibooker.setCurrentFolder("EcalDigisV/EcalDigiTask");

sprintf(histo, "EcalDigiTask Gun Momentum");
meGunEnergy_ = ibooker.book1D(histo, histo, 100, 0., 1000.);

sprintf(histo, "EcalDigiTask Gun Eta");
meGunEta_ = ibooker.book1D(histo, histo, 700, -3.5, 3.5);

sprintf(histo, "EcalDigiTask Gun Phi");
meGunPhi_ = ibooker.book1D(histo, histo, 360, 0., 360.);

sprintf(histo, "EcalDigiTask maximum Digi over Sim ratio");
meDigiSimRatio_ = ibooker.book1D(histo, histo, 100, 0., 2.);

sprintf(histo, "EcalDigiTask maximum Digi over Sim ratio gt 10 ADC");
meDigiSimRatiogt10ADC_ = ibooker.book1D(histo, histo, 100, 0., 2.);

sprintf(histo, "EcalDigiTask maximum Digi over Sim ratio gt 100 ADC");
meDigiSimRatiogt100ADC_ = ibooker.book1D(histo, histo, 100, 0., 2.);

sprintf(histo, "EcalDigiTask occupancy");
meDigiOccupancy_ = ibooker.book2D(histo, histo, 360, 0., 360., 170, -85., 85.);

sprintf(histo, "EcalDigiTask digis multiplicity");
meDigiMultiplicity_ = ibooker.book1D(histo, histo, 612, 0., 61200);

sprintf(histo, "EcalDigiTask global pulse shape");
meDigiADCGlobal_ = ibooker.bookProfile(histo, histo, kMaxSamples_, 0, kMaxSamples_, 10000, 0., 1000.);

for (int i = 0; i < kMaxSamples_; ++i) {
sprintf(histo, "EcalDigiTask analog pulse %02d", i + 1);
meDigiADCAnalog_[i] = ibooker.book1D(histo, histo, 4000, 0., 400.);

sprintf(histo, "EcalDigiTask ADC pulse %02d Gain 10", i + 1);
meDigiADCg10_[i] = ibooker.book1D(histo, histo, 4096, -0.5, 4095.5);

sprintf(histo, "EcalDigiTask ADC pulse %02d Gain 1", i + 1);
meDigiADCg1_[i] = ibooker.book1D(histo, histo, 4096, -0.5, 4095.5);

sprintf(histo, "EcalDigiTask gain pulse %02d", i + 1);
meDigiGain_[i] = ibooker.book1D(histo, histo, 2, 0, 2);
}

sprintf(histo, "EcalDigiTask pedestal for pre-sample");
mePedestal_ = ibooker.book1D(histo, histo, 4096, -0.5, 4095.5);

sprintf(histo, "EcalDigiTask maximum position gt 10 ADC");
meMaximumgt10ADC_ = ibooker.book1D(histo, histo, kMaxSamples_, 0., static_cast<double>(kMaxSamples_));

sprintf(histo, "EcalDigiTask maximum position gt 100 ADC");
meMaximumgt100ADC_ = ibooker.book1D(histo, histo, kMaxSamples_, 0., static_cast<double>(kMaxSamples_));

sprintf(histo, "EcalDigiTask ADC counts after gain switch");
menADCafterSwitch_ = ibooker.book1D(histo, histo, kMaxSamples_, 0., static_cast<double>(kMaxSamples_));
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, use std::string instead of a fixed sized buffer + sprintf. For the loop you can use std::format to concatenate an std::string with the counter.

meDigiADCGlobal_->Fill(sample, ebAnalogSignal[sample]);
}

if (ebSimMap[ebid.rawId()] != 0.) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::map::operator[] will silently create a key:value (with value 0.0f) pair in ebSimMap if ebid.rawId() is not found in the map. The initial filling of ebSimMap comes from looping over barrelHits (line 214), and here ebid.rawId() comes from looping over digis. You may consider the use of std::map:find(), to avoid filling ebSimMap silently.

#include <map>

class EcalDigisValidationPh2 : public DQMEDAnalyzer {
typedef std::map<uint32_t, float, std::less<uint32_t> > MapType;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::less<uint32_t> is already the default comparison object, you can remove it.

Comment on lines +67 to +106
void EcalTPsValidationPh2::bookHistograms(DQMStore::IBooker& ibooker, edm::Run const&, edm::EventSetup const&) {
Char_t histo[200];

ibooker.setCurrentFolder("EcalDigisV/EcalTriggerPrimitivesTask");

sprintf(histo, "EcalTPDigiTask TP digis multiplicity");
meTPDigisMultiplicity_ = ibooker.book1D(histo, histo, 613, 0, 61300);

sprintf(histo, "EcalTPDigiTask TP digi occupancy");
meTPDigiOccupancy_ = ibooker.book2D(histo, histo, 360, 0., 360., 170, -85., 85.);

sprintf(histo, "EcalTPDigiTask TP digi size");
meTPDigiSize_ = ibooker.book1D(histo, histo, kMaxSamples_, 0, kMaxSamples_);

sprintf(histo, "EcalTPDigiTask TP digi encoded ET");
meTPDigiEt_ = ibooker.book1D(histo, histo, 1025, -1, 1024);

sprintf(histo, "EcalTPDigiTask TP digi spike flag");
meTPDigiSpike_ = ibooker.book1D(histo, histo, 2, 0, 2);

sprintf(histo, "EcalTPDigiTask TP digi time");
meTPDigiTime_ = ibooker.book1D(histo, histo, 33, -1, 32);

sprintf(histo, "EcalTPDigiTask TP digi debug flag");
meTPDigiDebugFlag_ = ibooker.book1D(histo, histo, 2, 0, 2);

sprintf(histo, "EcalTPDigiTask TP digi sample of interest");
meTPDigiSOI_ = ibooker.book1D(histo, histo, kMaxSamples_ + 1, -1, kMaxSamples_);

for (unsigned int i = 0; i < kMaxSamples_; ++i) {
sprintf(histo, "EcalTPDigiTask TP sample %02d encoded ET", i);
meTPSampleEt_[i] = ibooker.book1D(histo, histo, 1024, 0, 1024);

sprintf(histo, "EcalTPDigiTask TP sample %02d spike flag", i);
meTPSampleSpike_[i] = ibooker.book1D(histo, histo, 2, 0, 2);

sprintf(histo, "EcalTPDigiTask TP sample %02d time", i);
meTPSampleTime_[i] = ibooker.book1D(histo, histo, 32, 0, 32);
}
}

@gabrielmscampos gabrielmscampos Jul 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here for std::string, forgot to comment before (sorry!)

@thomreis

thomreis commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

please test

@cmsbuild

cmsbuild commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51400/50118

  • There are other open Pull requests which might conflict with changes you have proposed:

@cmsbuild

cmsbuild commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Pull request #51400 was updated. @BenjaminRS, @ctarricone, @gabrielmscampos, @quinnanm, @rseidita can you please check and sign again.

@cmsbuild

cmsbuild commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

+1

Size: This PR adds an extra 28KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d37982/54589/summary.html
COMMIT: 4d1a752
CMSSW: CMSSW_20_1_X_2026-07-07-2300/el9_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/51400/54589/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 45
  • DQMHistoTests: Total histograms compared: 3547476
  • DQMHistoTests: Total failures: 107
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3547351
  • DQMHistoTests: Total skipped: 18
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 1509.69 KiB( 44 files compared)
  • DQMHistoSizes: changed ( 34434.0,... ): 337.924 KiB EcalDigisV/EcalTriggerPrimitivesTask
  • DQMHistoSizes: changed ( 34434.0,... ): -35.986 KiB EcalDigisV/EcalDigiTask
  • Checked 195 log files, 163 edm output root files, 45 DQM output files
  • TriggerResults: no differences found

@thomreis

thomreis commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Hi @gabrielmscampos can you please sign this again?

@gabrielmscampos

Copy link
Copy Markdown
Member

+dqm

@quinnanm

quinnanm commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

+l1

@cmsbuild

cmsbuild commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @mandrenguyen, @sextonkennedy, @ftenchini (and backports should be raised in the release meeting by the corresponding L2)

@mandrenguyen

Copy link
Copy Markdown
Contributor

+1

@cmsbuild
cmsbuild merged commit 7eb34e4 into cms-sw:master Jul 8, 2026
18 checks passed
@thomreis
thomreis deleted the ecal-phase2-digi-tp-val branch July 9, 2026 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants