Light by light modifier 1100 pre1 #27633

rchudasa · 2019-07-29T16:23:34Z

PR description:

This PR is submitted to reconstruct low pT photons down to 2 GeV for light by light scattering. It is just about relaxing few thresholds in python config with no changes in c++ source code. A modifier has been added to access the low pt photon reconstruction. Photon and electron efficiency plots and timing with default and modified reconstruction can be found in slides [1] presented in Reconstruction and Analysis Tools meeting. The modified reconstruction has been reviewed by Egamma POG convener @Sam-Harper .
[1]https://indico.cern.ch/event/836517/contributions/3513898/attachments/1887027/3111085/cms_reco_meeting_26thJuly_2019.pdf

cmsbuild · 2019-07-29T16:24:08Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-07-29T16:42:56Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27633/11126

This PR adds an extra 44KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Configuration/StandardSequences/python/Eras.py modified in PR(s): Phase2-hgx205 Add a new scenario for HGCal V11 geometry version #27555

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

code-checks:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27633/11126/code-checks.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27633/11126/code-checks.patch | patch -p1
You can also run scram build code-checks to apply code checks directly

slava77 · 2019-07-30T12:33:12Z

Configuration/Eras/python/Modifier_lightByLightLowPt_cff.py

@@ -0,0 +1,4 @@
+import FWCore.ParameterSet.Config as cms


Configuration/ProcessModifiers/python might be a better place

I tried to keep this modifier in ProcessModifiers repo as suggested by you and tried to run cmsDriver command with --era Run2_2018 --procModifiers lightByLightLowPt but it tries to find this Modifier in Eras repo, could you please suggest what else should I modify, so that cmsDriver command recognizes it from ProcessModifiers repo?

slava77 · 2019-07-30T12:47:00Z

Photon and electron efficiency plots and timing with default and modified reconstruction can be found in slides [1] presented in Reconstruction and Analysis Tools meeting. The modified reconstruction has been reviewed by Egamma POG convener @Sam-Harper .
[1]https://indico.cern.ch/event/836517/contributions/3513898/attachments/1887027/3111085/cms_reco_meeting_26thJuly_2019.pdf

I missed this part of the meeting.
What do you imply by the "default reco"?
Naively, given the 10_3_X context and HI PDs, I can guess that this is about pp_on_AA,
but I'm not sure.

Is this expected to be a once-in-CMS-lifetime for 10_3_X use case or should this be a supported workflow?

rchudasa · 2019-07-30T14:46:11Z

Photon and electron efficiency plots and timing with default and modified reconstruction can be found in slides [1] presented in Reconstruction and Analysis Tools meeting. The modified reconstruction has been reviewed by Egamma POG convener @Sam-Harper .
[1]https://indico.cern.ch/event/836517/contributions/3513898/attachments/1887027/3111085/cms_reco_meeting_26thJuly_2019.pdf

I missed this part of the meeting.
What do you imply by the "default reco"?
Naively, given the 10_3_X context and HI PDs, I can guess that this is about pp_on_AA,
but I'm not sure.

Is this expected to be a once-in-CMS-lifetime for 10_3_X use case or should this be a supported workflow?

The default reco is pp reconstruction with era "Run2_2018" and on top of that light by light modifier has been added. We don't use or modify pp_on_AA era as it modifies many Egamma thresholds to higher values which we don't want.
This is a supported workflow for higher CMSSW release and even for pp analyses, if required. :)

slava77 · 2019-07-30T15:05:39Z

The default reco is pp reconstruction with era "Run2_2018"

uhm, so this make a stronger case for backporting this at best to 10_6_X to be based on the UL release.

slava77 · 2019-08-01T13:52:09Z

@rchudasa
there is a merge conflict in this PR now
Please update.

cmsbuild · 2019-08-01T15:27:16Z

The code-checks are being triggered in jenkins.

rchudasa

@rchudasa
there is a merge conflict in this PR now
Please update.

@slava77 , I just committed a change now. Please note that I have not changed the location of lightbylight modifier to ProcessModifier repo yet, as cmsDriver command could not identify that.

rchudasa · 2019-07-31T13:45:15Z

Configuration/Eras/python/Modifier_lightByLightLowPt_cff.py

@@ -0,0 +1,4 @@
+import FWCore.ParameterSet.Config as cms


I tried to keep this modifier in ProcessModifiers repo as suggested by you and tried to run cmsDriver command with --era Run2_2018 --procModifiers lightByLightLowPt but it tries to find this Modifier in Eras repo, could you please suggest what else should I modify, so that cmsDriver command recognizes it from ProcessModifiers repo?

cmsbuild · 2019-08-01T15:35:03Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27633/11237

This PR adds an extra 44KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Configuration/StandardSequences/python/Eras.py modified in PR(s): Phase2-hcx214 Add a new scenario and workflow with latest Phase2 configuration including HFNose #27665

cmsbuild · 2019-08-12T18:51:44Z

+1
Tested at: fb097ff
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bda038/1975/summary.html

cmsbuild · 2019-08-12T18:51:48Z

Comparison job queued.

cmsbuild · 2019-08-12T21:07:40Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 34
DQMHistoTests: Total histograms compared: 2939508
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2939166
DQMHistoTests: Total skipped: 341
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
Checked 142 log files, 14 edm output root files, 34 DQM output files

slava77 · 2019-08-14T20:49:42Z

@rchudasa
compared to the test command in #27633 (comment)
I'm missing the event content modifications for the following in the latest version of the PR

"keep *_interestingGedEgammaIsoESDetId_*_*", "keep *_interestingGedEgammaIsoHCALDetId_*_*", 
"keep *_interestingGedEleIsoDetIdEB_*_*", "keep *_interestingGedEleIsoDetIdEE_*_*",
"keep *_interestingGedGamIsoDetIdEB_*_*","keep *_interestingGedGamIsoDetIdEE_*_*",
"keep EBDigiCollection_ecalDigis__*","keep EEDigiCollection_ecalDigis__*","keep ESDigiCollection_ecalPreshowerDigis__*",

Please check and confirm that these are not needed or update the PR to include the missing parts.

Also, compared to the test setup, the following were added

     'keep ZDCDataFramesSorted_hcalDigis_*_*',
    'keep ZDCDataFramesSorted_castorDigis_*_*',
    'keep QIE10DataFrameHcalDataFrameContainer_hcalDigis_ZDC_*'

ZDC data is small, so it seems OK to add if it's needed.

rchudasa · 2019-08-15T10:16:29Z

@rchudasa
compared to the test command in #27633 (comment)
I'm missing the event content modifications for the following in the latest version of the PR
"keep *_interestingGedEgammaIsoESDetId_*_*", "keep *_interestingGedEgammaIsoHCALDetId_*_*", 
"keep *_interestingGedEleIsoDetIdEB_*_*", "keep *_interestingGedEleIsoDetIdEE_*_*",
"keep *_interestingGedGamIsoDetIdEB_*_*","keep *_interestingGedGamIsoDetIdEE_*_*",
"keep EBDigiCollection_ecalDigis__*","keep EEDigiCollection_ecalDigis__*","keep ESDigiCollection_ecalPreshowerDigis__*",
Please check and confirm that these are not needed or update the PR to include the missing parts.

Also, compared to the test setup, the following were added
     'keep ZDCDataFramesSorted_hcalDigis_*_*',
    'keep ZDCDataFramesSorted_castorDigis_*_*',
    'keep QIE10DataFrameHcalDataFrameContainer_hcalDigis_ZDC_*'
ZDC data is small, so it seems OK to add if it's needed.

I consulted @Sam-Harper and concluded that we dont need those event contents as they are already present in reducedEgamma rechit collection. We require ZDC event content for the analysis.

slava77 · 2019-08-15T12:18:20Z

I consulted @Sam-Harper and concluded that we dont need those event contents as they are already present in reducedEgamma rechit collection. We require ZDC event content for the analysis.

Do you really mean "reducedEgamma"? This is a module from miniAOD.
In the earlier discussion we concluded that you will not run/need miniAOD.

Perhaps you actually mean other reduced rechit collections available in AOD.
Please clarify/confirm.

rchudasa · 2019-08-15T13:59:18Z

I consulted @Sam-Harper and concluded that we dont need those event contents as they are already present in reducedEgamma rechit collection. We require ZDC event content for the analysis.

Do you really mean "reducedEgamma"? This is a module from miniAOD.
In the earlier discussion we concluded that you will not run/need miniAOD.

Perhaps you actually mean other reduced rechit collections available in AOD.
Please clarify/confirm.

Sorry, for the confusion. We will use AOD content and to be precise, reducedEcalRecHitsEB, reducedEcalRecHitsEE, reducedEcalRecHitsES collections.

slava77 · 2019-08-15T20:38:36Z

+1

for #27633 fb097ff

code changes are in line with the PR description and the follow up review
jenkins tests pass and comparisons with the baseline show no differences (the new process modifier is not exercised)
the latest updates should not change conclusions made in test in Light by light modifier 1100 pre1 #27633 (comment) (or in the PR description). Some moderate cleanup of the keep statements should reduce the event size by ~1 kB/event to just under 30 kB/evt (compared to an increase from 19 to 31 kiB observed in the earlier tests)

slava77 · 2019-08-19T08:19:32Z

@fabiocos @kpedro88
please check this and comment or merge some time soon.
Thank you.

kpedro88 · 2019-08-22T21:25:09Z

+1
At some point (once decisions on production are made), a workflow using this modifier should be put into the matrix.

kpedro88 · 2019-08-22T21:26:02Z

merge

fabiocos · 2019-09-02T08:09:19Z

+operations

the addition of the modifier is correct, although it is not activated by default in a test workflow

cmsbuild · 2019-09-02T08:09:46Z

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 be automatically merged.

cmsbuild added this to the CMSSW_11_0_X milestone Jul 29, 2019

cmsbuild added code-checks-pending comparison-pending operations-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Jul 29, 2019

cmsbuild added code-checks-rejected and removed code-checks-pending labels Jul 29, 2019

slava77 reviewed Jul 30, 2019

View reviewed changes

slava77 mentioned this pull request Jul 30, 2019

Backporting EG UL regression code to 1030 #27642

Closed

Sam-Harper and others added 4 commits July 31, 2019 12:35

adding light by light modifier

6b8c8ad

updating rechits and ecal digis selectors

7929e6a

Modifier for lbyl added to Eras repo

ce82fec

interesting rechit threshold updated

bf4ecb0

cmsbuild mentioned this pull request Aug 1, 2019

Phase2-hcx214 Add a new scenario and workflow with latest Phase2 configuration including HFNose #27665

Merged

ecalDrivenElectronseed merge conflict solved

645b619

rchudasa force-pushed the LightByLightModifier_1100_pre1 branch from e7b200e to 645b619 Compare August 1, 2019 15:26

cmsbuild added code-checks-pending and removed code-checks-rejected labels Aug 1, 2019

rchudasa commented Aug 1, 2019

View reviewed changes

cmsbuild added the tests-started label Aug 12, 2019

cmsbuild added tests-approved and removed tests-started labels Aug 12, 2019

cmsbuild added comparison-available and removed comparison-pending labels Aug 12, 2019

cmsbuild mentioned this pull request Aug 15, 2019

Reconfigure particle flow cfi #27782

Closed

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Aug 15, 2019

cmsbuild mentioned this pull request Aug 19, 2019

Toplhefilter #27795

Closed

cmsbuild added orp-approved and removed orp-pending labels Aug 22, 2019

cmsbuild merged commit b5741be into cms-sw:master Aug 22, 2019

rchudasa mentioned this pull request Aug 29, 2019

Backport: Light by light modifier PR 103X #27898

Merged

cmsbuild added fully-signed operations-approved and removed operations-pending pending-signatures labels Sep 2, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Light by light modifier 1100 pre1 #27633

Light by light modifier 1100 pre1 #27633

rchudasa commented Jul 29, 2019

cmsbuild commented Jul 29, 2019

cmsbuild commented Jul 29, 2019

slava77 Jul 30, 2019

rchudasa Jul 31, 2019

slava77 commented Jul 30, 2019

rchudasa commented Jul 30, 2019

slava77 commented Jul 30, 2019

slava77 commented Aug 1, 2019

cmsbuild commented Aug 1, 2019

rchudasa left a comment

rchudasa Jul 31, 2019

cmsbuild commented Aug 1, 2019

cmsbuild commented Aug 12, 2019

cmsbuild commented Aug 12, 2019

cmsbuild commented Aug 12, 2019

slava77 commented Aug 14, 2019

rchudasa commented Aug 15, 2019

slava77 commented Aug 15, 2019

rchudasa commented Aug 15, 2019

slava77 commented Aug 15, 2019

slava77 commented Aug 19, 2019

kpedro88 commented Aug 22, 2019

kpedro88 commented Aug 22, 2019

fabiocos commented Sep 2, 2019

cmsbuild commented Sep 2, 2019

Light by light modifier 1100 pre1 #27633

Light by light modifier 1100 pre1 #27633

Conversation

rchudasa commented Jul 29, 2019

PR description:

cmsbuild commented Jul 29, 2019

cmsbuild commented Jul 29, 2019

slava77 Jul 30, 2019

Choose a reason for hiding this comment

rchudasa Jul 31, 2019

Choose a reason for hiding this comment

slava77 commented Jul 30, 2019

rchudasa commented Jul 30, 2019

slava77 commented Jul 30, 2019

slava77 commented Aug 1, 2019

cmsbuild commented Aug 1, 2019

rchudasa left a comment

Choose a reason for hiding this comment

rchudasa Jul 31, 2019

Choose a reason for hiding this comment

cmsbuild commented Aug 1, 2019

cmsbuild commented Aug 12, 2019

cmsbuild commented Aug 12, 2019

cmsbuild commented Aug 12, 2019

slava77 commented Aug 14, 2019

rchudasa commented Aug 15, 2019

slava77 commented Aug 15, 2019

rchudasa commented Aug 15, 2019

slava77 commented Aug 15, 2019

slava77 commented Aug 19, 2019

kpedro88 commented Aug 22, 2019

kpedro88 commented Aug 22, 2019

fabiocos commented Sep 2, 2019

cmsbuild commented Sep 2, 2019