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

Associated PF for Egamma in 2018 PbPb miniAOD #37262

Merged
merged 5 commits into from
Apr 25, 2022

Conversation

ttrk
Copy link
Contributor

@ttrk ttrk commented Mar 17, 2022

Description

PF candidate association is necessary for a GED electron/photon to avoid double counting energy in PF isolation.

In AOD, the association is stored in particleBasedIsolation object. In miniAOD, association is accessible via associatedPackedPFCandidates() of a pat electron/photon (e.g. here), the AOD-2-miniAOD process transfers the association between objects.

Currently, the transfer does not work correctly for 2018 PbPb data, i.e. run2_miniAOD_pp_on_AA_103X process. This is understood as packedPFcandidates in this workflow is made from an intermediate, "cleaned" PF collection (introduced via PR) and while references in particleBasedIsolation are still the ones from original PF collection. The associatedPackedPFCandidates collection created in current workflow is not correct.

This PR fixes the problem by producing a map between original (old) and the cleaned (new) PF candidate references and updating the references in particleBasedIsolation from old to new ones.

Validation
Tests pass wf 140.5611 and give correct output.

@mandrenguyen

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37262/28885

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

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37262/28887

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @ttrk (Kaya Tatar) for master.

It involves the following packages:

  • PhysicsTools/PatAlgos (reconstruction)
  • RecoEgamma/EgammaPhotonProducers (reconstruction)
  • RecoHI/HiJetAlgos (reconstruction)

@jpata, @cmsbuild, @clacaputo, @slava77 can you please review it and eventually sign? Thanks.
@emilbols, @gouskos, @yslai, @jainshilpi, @hatakeyamak, @rappoccio, @mbluj, @demuller, @varuns23, @seemasharmafnal, @mmarionncern, @ahinzmann, @lgray, @jdolen, @sobhatta, @azotz, @mandrenguyen, @yetkinyilmaz, @Sam-Harper, @kurtejung, @rovere, @jdamgov, @nhanvtran, @gkasieczka, @schoef, @dgulhan, @AlexDeMoor, @valsdav, @jazzitup, @JyothsnaKomaragiri, @yenjie, @lecriste, @afiqaize, @gpetruc, @wrtabb, @mariadalfonso, @andrzejnovak, @ram1123 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

@clacaputo
Copy link
Contributor

@cmsbuild please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f7117e/23252/summary.html
COMMIT: 68ac690
CMSSW: CMSSW_12_4_X_2022-03-21-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/37262/23252/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

The relvals timed out after 4 hours.

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-f7117e/39434.911_TTbar_14TeV+2026D88_DD4hep+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 2 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3695650
  • DQMHistoTests: Total failures: 51289
  • DQMHistoTests: Total nulls: 67
  • DQMHistoTests: Total successes: 3644272
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.004 KiB( 48 files compared)
  • DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
  • Checked 204 log files, 45 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@clacaputo
Copy link
Contributor

Hi @smuzaffar , it is not clear to me whether the time out error is related to the PR or to jenkins

@smuzaffar
Copy link
Contributor

@clacaputo , timeout is not related to this PR. the Relval INPUT tests try to run all workflows step1 . Looks like due to network issue that test timedout of 4 hour ( https://cmssdt.cern.ch/jenkins/job/ib-run-pr-relvals/17127/ ). I would suggest to ignore this error

@clacaputo
Copy link
Contributor

test parameters:

  • workflow = 140.5611

@clacaputo
Copy link
Contributor

@cmsbuild please test

@cmsbuild
Copy link
Contributor

Pull request #37262 was updated. @jpata, @cmsbuild, @clacaputo, @slava77 can you please check and sign again.

@clacaputo
Copy link
Contributor

@cmsbuild please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f7117e/23998/summary.html
COMMIT: a9fc49b
CMSSW: CMSSW_12_4_X_2022-04-18-2300/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/37262/23998/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: 6 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3594584
  • DQMHistoTests: Total failures: 7
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 3594554
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.004 KiB( 48 files compared)
  • DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
  • Checked 203 log files, 46 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@clacaputo
Copy link
Contributor

+reconstruction

  • resign

@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)

@qliphy
Copy link
Contributor

qliphy commented Apr 25, 2022

please test
to refresh

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f7117e/24172/summary.html
COMMIT: a9fc49b
CMSSW: CMSSW_12_4_X_2022-04-24-0000/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/37262/24172/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-f7117e/39434.75_TTbar_14TeV+2026D88_HLT75e33+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HLT75e33+HARVESTGlobal

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 3700081
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3700057
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
  • Checked 208 log files, 46 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

@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.

8 participants