Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HCAL: Applying the fix on pulse containment correction algorithm both for Run3 trigger primitives and Method 0 #35762

Merged
merged 2 commits into from Oct 26, 2021

Conversation

TaeunKwon
Copy link
Contributor

PR description:

In the PR #34572 and #34805, the pulse containment correction(PCC) algorithm was updated to make the timePhase definition in HcalPulseContainment similar to SIM step for Run3 Trigger Primitives(TPs). This update was originally designed to make a better agreement between TP energy for the PFA1' scheme and MAHI energy, but recently we found that it also has affected TP energy for the PFA2 scheme(old TP algorithm), by reducing its energy scale by 3~4%. And because we haven't applied this update in the PCC algorithm for RecHits(Method 0), currently there exists a discrepancy between PFA2 and Method 0, which we don't want. Therefore, this PR applies the update on the PCC algorithm described in PR #34572 also for method 0, by adding a shared configurable parameter switching on the PCC update both in TP and Method 0. The impact of this PR is shown in this presentation.

PR validation:

A basic technical test was performed: runTheMatrix.py -l limited

if this PR is a backport please specify the original PR and why you need to backport that PR:

This PR is not a backport.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35762/26106

  • This PR adds an extra 24KB to repository

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • CalibCalorimetry/HcalPlugins (alca)
  • CalibCalorimetry/HcalTPGEventSetup (l1, alca)
  • RecoLocalCalo/HcalRecAlgos (reconstruction)
  • RecoLocalCalo/HcalRecProducers (reconstruction)
  • SimCalorimetry/HcalTrigPrimProducers (l1)

@malbouis, @yuanchao, @cmsbuild, @rekovic, @slava77, @jpata, @tvami, @cecilecaillol, @francescobrivio can you please review it and eventually sign? Thanks.
@bsunanda, @rchatter, @rovere, @argiro, @apsallid, @tocheng, @thomreis, @simonepigazzini, @mmusich, @abdoulline, @mariadalfonso this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@tvami
Copy link
Contributor

tvami commented Oct 21, 2021

@cmsbuild , please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-547dbb/19793/summary.html
COMMIT: d30db88
CMSSW: CMSSW_12_1_X_2021-10-20-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35762/19793/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: 76 differences found in the comparisons
  • DQMHistoTests: Total files compared: 40
  • DQMHistoTests: Total histograms compared: 2751113
  • DQMHistoTests: Total failures: 100
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2750990
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.004 KiB( 39 files compared)
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 170 log files, 37 edm output root files, 40 DQM output files
  • TriggerResults: no differences found

@tvami
Copy link
Contributor

tvami commented Oct 21, 2021

+alca

  • changes in AlCa files are trivial, just introduce a new boolean and it's switched on for Run-3
  • Jenkins diffs are visible in HCAL, which is expected (+MsgLogger)

@slava77
Copy link
Contributor

slava77 commented Oct 23, 2021

+reconstruction

for #35762 d30db88

Co-authored-by: Slava Krutelyov <slava77@gmail.com>
@cmsbuild
Copy link
Contributor

Pull request #35762 was updated. @malbouis, @yuanchao, @rekovic, @slava77, @jpata, @tvami, @cecilecaillol, @francescobrivio can you please check and sign again.

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-547dbb/19903/summary.html
COMMIT: 1cbf578
CMSSW: CMSSW_12_1_X_2021-10-24-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35762/19903/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

----- Begin Fatal Exception 25-Oct-2021 16:44:28 CEST-----------------------
An exception of category 'NoRecord' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 1 stream: 0
   [1] Running path 'prevalidation_step'
   [2] Prefetching for module MultiTrackValidator/'trackValidatorBuilding'
   [3] Prefetching for module TrackProducer/'detachedTripletStepTracks'
   [4] Prefetching for module TrackCandidateProducer/'detachedTripletStepTrackCandidates'
   [5] Prefetching for module FastTrackerRecHitMaskProducer/'detachedTripletStepMasks'
   [6] Prefetching for module TrackProducer/'detachedQuadStepTracks'
   [7] Prefetching for module TrackCandidateProducer/'detachedQuadStepTrackCandidates'
   [8] Prefetching for module FastTrackerRecHitMaskProducer/'detachedQuadStepMasks'
   [9] Prefetching for module TrackProducer/'lowPtTripletStepTracks'
   [10] Prefetching for module TrackCandidateProducer/'lowPtTripletStepTrackCandidates'
   [11] Prefetching for module FastTrackerRecHitMaskProducer/'lowPtTripletStepMasks'
   [12] Prefetching for module TrackProducer/'highPtTripletStepTracks'
   [13] Prefetching for module TrackCandidateProducer/'highPtTripletStepTrackCandidates'
   [14] Prefetching for module FastTrackerRecHitMaskProducer/'highPtTripletStepMasks'
   [15] Prefetching for module TrackProducer/'lowPtQuadStepTracks'
   [16] Prefetching for module TrackCandidateProducer/'lowPtQuadStepTrackCandidates'
   [17] Prefetching for module FastTrackerRecHitMaskProducer/'lowPtQuadStepMasks'
   [18] Calling method for module TrackTfClassifier/'initialStep'
Exception Message:
No "TfGraphRecord" record found in the EventSetup.

 The Record is delivered by an ESSource or ESProducer but there is no valid IOV for the synchronization value.
 Please check 
   a) if the synchronization value is reasonable and report to the hypernews if it is not.
   b) else check that all ESSources have been properly configured. 
----- End Fatal Exception -------------------------------------------------

@perrotta
Copy link
Contributor

@cmsbuild , please test with #35801

  • You were right @tvami , CMSSW_12_1_X_2021-10-25-1100 was not picked up yet by the tests

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-547dbb/19923/summary.html
COMMIT: 1cbf578
CMSSW: CMSSW_12_1_X_2021-10-25-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35762/19923/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: 70 differences found in the comparisons
  • DQMHistoTests: Total files compared: 41
  • DQMHistoTests: Total histograms compared: 2797338
  • DQMHistoTests: Total failures: 99
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2797216
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.004 KiB( 40 files compared)
  • DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
  • Checked 173 log files, 37 edm output root files, 41 DQM output files
  • TriggerResults: no differences found

@rekovic
Copy link
Contributor

rekovic commented Oct 26, 2021

+1

@slava77
Copy link
Contributor

slava77 commented Oct 26, 2021

+reconstruction

for #35762 1cbf578

  • code changes since the last reco signoff were only in CalibCalorimetry/HcalPlugins

@cmsbuild
Copy link
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. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@perrotta
Copy link
Contributor

+1

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.

None yet

6 participants