Updated EcalUncalibRecHitTimingCCAlgo to correct for bias at high energies #45386

jking79 · 2024-07-05T10:39:06Z

Added correction for MPGA slew issue to CC time reconstruction algorithm
Added useSlewCorrectionEB and useSlewCorrectionEE parameters to activate slew correction code
Tested and presented in ECAL DPG : https://indico.cern.ch/event/1433850/

…EE v2

cmsbuild · 2024-07-05T10:39:32Z

cms-bot internal usage

cmsbuild · 2024-07-05T10:41:10Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45386/40835

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-45386/40835/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45386/40835/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

thomreis · 2024-07-05T10:42:00Z

type ecal

…checks/cms-sw-PR-45386/40835/code-format.patch

cmsbuild · 2024-07-05T10:46:10Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45386/40836

cmsbuild · 2024-07-05T10:46:32Z

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

It involves the following packages:

RecoLocalCalo/EcalRecAlgos (reconstruction)
RecoLocalCalo/EcalRecProducers (reconstruction)

@cmsbuild, @jfernan2, @mandrenguyen can you please review it and eventually sign? Thanks.
@ReyerBand, @apsallid, @argiro, @missirol, @rchatter, @thomreis, @wang0jin, @youyingli this is something you requested to watch as well.
@antoniovilela, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

thomreis · 2024-07-05T10:54:43Z

RecoLocalCalo/EcalRecAlgos/src/EcalUncalibRecHitTimingCCAlgo.cc

-    powerSamples += std::pow(samples[i], 2);
-    powerTemplate += std::pow(interpolated[i], 2);
-    cc += interpolated[i] * samples[i];
+    powerSamples += std::pow(samples[i], 2) * weights[i];


Not important now since the weights are either 0 or 1, but would it make more sense to have std::pow(samples[i] * weights[i], 2) instead when there may be float values for weights?

thomreis · 2024-07-05T10:54:59Z

RecoLocalCalo/EcalRecProducers/python/ecalMultiFitUncalibRecHit_cfi.py

@@ -14,6 +14,8 @@
        outOfTimeThresholdGain12mEB = 3.0,
        outOfTimeThresholdGain61pEB = 3.0,
        outOfTimeThresholdGain61mEB = 3.0,
+        useSlewCorrectionEB = True,
+        useSlewCorrectionEE = False,


I think these are not needed.

thomreis · 2024-07-05T10:58:23Z

RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerMultiFit.cc

@@ -783,6 +794,8 @@ edm::ParameterSetDescription EcalUncalibRecHitWorkerMultiFit::getAlgoDescription
              edm::ParameterDescription<double>("outOfTimeThresholdGain61mEE", 1000, true) and
              edm::ParameterDescription<double>("amplitudeThresholdEB", 10, true) and
              edm::ParameterDescription<double>("amplitudeThresholdEE", 10, true) and
+              edm::ParameterDescription<bool>("useSlewCorrectionEB", true, true) and


Maybe change the variable name to crossCorrelationUseSlewCorrectionEB to distinguish it clearly from the slew rate fixes in the amplitude reconstruction, which are activated with the gainSwitchUseMaxSampleEB parameter.

…fi.py, Changed name of slew correction paramters in EcalUncalibRecHitWorkerMultiFit.cc

cmsbuild · 2024-07-05T12:46:05Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45386/40838

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-45386/40838/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45386/40838/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

…code-format.patch

thomreis · 2024-07-05T15:02:27Z

please test

Errors do not seem related to code changes.

cmsbuild · 2024-07-05T17:42:31Z

-1

Failed Tests: RelVals
Size: This PR adds an extra 12KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-01bead/40265/summary.html
COMMIT: 15b297b
CMSSW: CMSSW_14_1_X_2024-07-04-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/45386/40265/install.sh to create a dev area with all the needed externals and cmssw changes.

DAS Queries: The DAS query tests failed, see the summary page for details.

RelVals

29634.911A fatal system signal has occurred: segmentation violation

thomreis · 2024-07-05T20:32:38Z

The same segfault happens also in the tests of #45375
@antoniovilela @rappoccio is there a general issue with the tests at the moment?

thomreis · 2024-07-08T08:02:51Z

please test

cmsbuild · 2024-07-08T11:36:58Z

+1

Size: This PR adds an extra 12KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-01bead/40278/summary.html
COMMIT: 15b297b
CMSSW: CMSSW_14_1_X_2024-07-07-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/45386/40278/install.sh to create a dev area with all the needed externals and cmssw changes.

DAS Queries: The DAS query tests failed, see the summary page for details.

Comparison Summary

Summary:

You potentially added 7 lines to the logs
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3345088
DQMHistoTests: Total failures: 3
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3345065
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 202 log files, 165 edm output root files, 48 DQM output files
TriggerResults: no differences found

jfernan2 · 2024-07-08T14:08:11Z

+1

cmsbuild · 2024-07-08T14:08:38Z

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. @sextonkennedy, @antoniovilela, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

antoniovilela · 2024-07-08T22:32:54Z

The same segfault happens also in the tests of #45375 @antoniovilela @rappoccio is there a general issue with the tests at the moment?

Thomas, please use @cms-sw/orp-l2 to mention us.

antoniovilela · 2024-07-08T22:33:09Z

+1

jking79 added 2 commits July 5, 2024 04:53

Slew Correction fo CC + parameters to use this correction in EB &/or EE

bb21a09

Slew Correction fo CC + parameters to use this correction in EB &/or …

173f02c

…EE v2

cmsbuild added this to the CMSSW_14_1_X milestone Jul 5, 2024

cmsbuild added reconstruction-pending pending-signatures tests-pending orp-pending code-checks-pending labels Jul 5, 2024

cmsbuild added code-checks-rejected and removed code-checks-pending labels Jul 5, 2024

cmsbuild added the ecal label Jul 5, 2024

Applied code style and quality patch https://cmssdt.cern.ch/SDT/code-…

22924d4

…checks/cms-sw-PR-45386/40835/code-format.patch

cmsbuild added code-checks-pending and removed code-checks-rejected labels Jul 5, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 5, 2024

thomreis reviewed Jul 5, 2024

View reviewed changes

Removed useSlewCorrection parameters from ecalMultiFitUncalibRecHit_c…

1348593

…fi.py, Changed name of slew correction paramters in EcalUncalibRecHitWorkerMultiFit.cc

cmsbuild added code-checks-pending and removed code-checks-approved labels Jul 5, 2024

jking79 changed the title ~~Pr master cc slew correction~~ Updated EcalUncalibRecHitTimingCCAlgo to correct for bias at high energies Jul 5, 2024

cmsbuild added code-checks-rejected and removed code-checks-pending labels Jul 5, 2024

Applied https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45386/40838/…

15b297b

…code-format.patch

cmsbuild added tests-rejected and removed tests-started labels Jul 5, 2024

cmsbuild added tests-started and removed tests-rejected labels Jul 5, 2024

cmsbuild added tests-rejected and removed tests-started labels Jul 5, 2024

cmsbuild added tests-started and removed tests-rejected labels Jul 8, 2024

cmsbuild added tests-approved and removed tests-started labels Jul 8, 2024

cmsbuild added reconstruction-approved fully-signed and removed reconstruction-pending pending-signatures labels Jul 8, 2024

cmsbuild added orp-approved and removed orp-pending labels Jul 8, 2024

cmsbuild merged commit 3b0667b into cms-sw:master Jul 8, 2024
20 checks passed

This was referenced Jul 9, 2024

Set c++20 as a default standard cms-sw/cmsdist#9288

Merged

Explicitly initialize bTag #45409

Merged

[GCC13] Update gcc to 13.3.0 cms-sw/cmsdist#9270

Open

[GCC13] Patch applied for c++20 cms-sw/cmsdist#9295

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updated EcalUncalibRecHitTimingCCAlgo to correct for bias at high energies #45386

Updated EcalUncalibRecHitTimingCCAlgo to correct for bias at high energies #45386

jking79 commented Jul 5, 2024

cmsbuild commented Jul 5, 2024 •

edited

Loading

cmsbuild commented Jul 5, 2024

thomreis commented Jul 5, 2024

cmsbuild commented Jul 5, 2024

cmsbuild commented Jul 5, 2024

thomreis Jul 5, 2024

thomreis Jul 5, 2024

thomreis Jul 5, 2024

cmsbuild commented Jul 5, 2024

thomreis commented Jul 5, 2024

cmsbuild commented Jul 5, 2024

thomreis commented Jul 5, 2024

thomreis commented Jul 8, 2024

cmsbuild commented Jul 8, 2024

jfernan2 commented Jul 8, 2024

cmsbuild commented Jul 8, 2024

antoniovilela commented Jul 8, 2024

antoniovilela commented Jul 8, 2024

Updated EcalUncalibRecHitTimingCCAlgo to correct for bias at high energies #45386

Updated EcalUncalibRecHitTimingCCAlgo to correct for bias at high energies #45386

Conversation

jking79 commented Jul 5, 2024

cmsbuild commented Jul 5, 2024 • edited Loading

cmsbuild commented Jul 5, 2024

thomreis commented Jul 5, 2024

cmsbuild commented Jul 5, 2024

cmsbuild commented Jul 5, 2024

thomreis Jul 5, 2024

Choose a reason for hiding this comment

thomreis Jul 5, 2024

Choose a reason for hiding this comment

thomreis Jul 5, 2024

Choose a reason for hiding this comment

cmsbuild commented Jul 5, 2024

thomreis commented Jul 5, 2024

cmsbuild commented Jul 5, 2024

RelVals

thomreis commented Jul 5, 2024

thomreis commented Jul 8, 2024

cmsbuild commented Jul 8, 2024

Comparison Summary

jfernan2 commented Jul 8, 2024

cmsbuild commented Jul 8, 2024

antoniovilela commented Jul 8, 2024

antoniovilela commented Jul 8, 2024

cmsbuild commented Jul 5, 2024 •

edited

Loading