Skip to content

Conversation

@abdoulline
Copy link

@abdoulline abdoulline commented Jun 7, 2020

PR description:

Existing for Phase2 customization to exclude HBHEIsolatedNoiseReflagger (only relevant for Phase0 HPDs HCAL subdetectors), formerly known as "hbhereco" producer, has been backported to Run3 to save CPU and disk space: one HBHERecHits collection "hbhereco" now produced instead of two "hbheprereco" + "hbhereco" (effectively identical for >= Run3) previously.

NB: HLT for Run3 is already using solely "hbhereco"=HBHEPhase1Reconstructor.

PR validation:

runTheMatrix.py -l limited OK

No backport needed

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 7, 2020

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 7, 2020

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30138/15924

  • This PR adds an extra 12KB to repository

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 7, 2020

A new Pull Request was created by @abdoulline (Salavat Abdullin) for master.

It involves the following packages:

RecoJets/JetProducers
RecoLocalCalo/Configuration

@perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@rappoccio, @apsallid, @jdolen, @ahinzmann, @rovere, @jdamgov, @yslai, @nhanvtran, @gkasieczka, @clelange, @lecriste, @schoef, @mariadalfonso, @seemasharmafnal this is something you requested to watch as well.
@silviodonato, @dpiparo you are the release manager for this.

cms-bot commands are listed here

@abdoulline abdoulline mentioned this pull request Jun 7, 2020
@slava77
Copy link
Contributor

slava77 commented Jun 8, 2020

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2020

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/6867/console Started: 2020/06/08 02:05

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2020

+1
Tested at: 59c44f6
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6555e3/6867/summary.html
CMSSW: CMSSW_11_2_X_2020-06-07-0000
SCRAM_ARCH: slc7_amd64_gcc820

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2020

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2020

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6555e3/6867/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 24 differences found in the comparisons
  • DQMHistoTests: Total files compared: 36
  • DQMHistoTests: Total histograms compared: 2780497
  • DQMHistoTests: Total failures: 74
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2780373
  • DQMHistoTests: Total skipped: 50
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 35 files compared)
  • Checked 152 log files, 16 edm output root files, 36 DQM output files

@abdoulline
Copy link
Author

abdoulline commented Jun 8, 2020

Some difference observed only in old (obsolete ?) GlobalRecHitsV
(with getManyByType(hbhe) and interation over all collections [1])
for 11634.0 (2021) and 12434.0 (2023) wfs.

Possible explanation: this obsolete analyser [1] picks up and iterate over all existing HBHERecHits collections. Previously there were three of them: (i) hbheprereco, (ii) hbhereco and (iii)"reducedHcalRecHits" (wih label "hbhereco").
Now remains only the latter two. In addition, (iii) becomes smaller, as RecHits with redundant (set, but not used for Run3, Phase2 ) "HBHEIsolatedNoise" flag [2] are not added to this collection anymore.


[1]
https://cmssdt.cern.ch/lxr/source/Validation/GlobalRecHits/src/GlobalRecHitsProducer.cc#0526

[2]
https://cmssdt.cern.ch/lxr/source/DataFormats/METReco/interface/HcalPhase1FlagLabels.h#0009

@abdoulline
Copy link
Author

abdoulline commented Jun 8, 2020

Will follow (to be added to this PR) later today:
HCAL Run3-related DQM configs adjustment (move of existing Phase2 customization to Run3)

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2020

Pull request #30138 was updated. @perrotta, @andrius-k, @kmaeshima, @schneiml, @cmsbuild, @jfernan2, @fioriNTU, @slava77 can you please check and sign again.

@slava77
Copy link
Contributor

slava77 commented Jun 8, 2020

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2020

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/6875/console Started: 2020/06/08 15:58

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2020

+1
Tested at: 98a7464
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6555e3/6875/summary.html
CMSSW: CMSSW_11_2_X_2020-06-08-1100
SCRAM_ARCH: slc7_amd64_gcc820

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2020

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2020

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6555e3/6875/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 20 differences found in the comparisons
  • DQMHistoTests: Total files compared: 36
  • DQMHistoTests: Total histograms compared: 2779875
  • DQMHistoTests: Total failures: 33
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2779792
  • DQMHistoTests: Total skipped: 50
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -416.46 KiB( 35 files compared)
  • DQMHistoSizes: changed ( 11634.0,... ): -198.366 KiB JetMET/MET
  • DQMHistoSizes: changed ( 11634.0,... ): -9.864 KiB HcalNoiseRatesD/HcalNoiseRatesTask
  • Checked 152 log files, 16 edm output root files, 36 DQM output files

@jfernan2
Copy link
Contributor

jfernan2 commented Jun 9, 2020

Thanks @abdoulline for this cleaning.
There is a small reduction in number of entries for 4/5 plots, not sure if relevant. I ask you just in case it is not expected:
https://tinyurl.com/y9vaf5n3
https://tinyurl.com/ydz988jl

@abdoulline
Copy link
Author

Thanks @abdoulline for this cleaning.
There is a small reduction in number of entries for 4/5 plots, not sure if relevant. I ask you just in case it is not expected:
https://tinyurl.com/y9vaf5n3
https://tinyurl.com/ydz988jl

@jfernan2
Yes, I've commented on them earlier in this thread (after the first round, yet without DQM changes),
giving an explanation: #30138 (comment)

@jfernan2
Copy link
Contributor

jfernan2 commented Jun 9, 2020

Thanks @abdoulline Sorry, I had understood your last commit would fix that too. OK

@jfernan2
Copy link
Contributor

jfernan2 commented Jun 9, 2020

+1

@perrotta
Copy link
Contributor

+1

  • Removed redundant collection from Run3 HCal local reco
  • Jenkins tests pass: the reco outputs for Run3 wfs witness the removal of "hbheprereco", as desired; no other changes are observed

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

@silviodonato
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit db2015d into cms-sw:master Jun 12, 2020
@abdoulline abdoulline deleted the remove_HBHEIsolatedNoiseReflagger_fromRun3 branch June 6, 2021 17:30
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