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

pp RECO with HI photon isolation using "customise" option #12185

Merged
merged 2 commits into from Nov 5, 2015

Conversation

ttrk
Copy link
Contributor

@ttrk ttrk commented Oct 29, 2015

This PR adds the HI-style isolation to the pp photon sequences. Heavy Ions needs this information during the pp reference run in order to do proper comparisons to PbPb results, where this information will be used for photon ID.

It will add two instances of the HIPhotonIsolation ValueMap, one for "std" photons and another for GED photons to the RECO and AOD event content for pp events.
This is urgent, as the pp reference run is approaching soon.

As pointed by @slava77 in the RECO meeting on 29.10.2015, this PR is planned to go into 75X.

Upon feedback on the previous PR, this PR implements "customisation" approach.

@richard-cms
@yenjie

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @ttrk for CMSSW_7_5_X.

pp RECO with HI photon isolation using "customise" option

It involves the following packages:

RecoHI/Configuration
RecoHI/HiEgammaAlgos

@cmsbuild, @cvuosalo, @slava77 can you please review it and eventually sign? Thanks.
@MiheeJo, @jazzitup, @richard-cms, @echapon, @yenjie, @kurtejung, @istaslis, @mandrenguyen, @dgulhan, @yetkinyilmaz this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
If you are a L2 or a release manager you can ask for tests by saying 'please test' or '@cmsbuild, please test' in the first line of a comment.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@slava77
Copy link
Contributor

slava77 commented Oct 30, 2015

@cmsbuild please test

@davidlange6 I see the last auto-merge 75X->76X on Oct 18. Did we stop since then?
(I think yes, but I'm not sure)

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

The jenkins tests job failed, please try again.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/9393/console

@harmonicoscillator
Copy link
Contributor

I think Jenkins had a hiccup here, https://cmssdt.cern.ch/jenkins/job/ib-any-integration/9393/console is inaccessible.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@harmonicoscillator
Copy link
Contributor

Is there any more information or changes you'd like to see from me or @ttrk?

Should we include our customization in postLS1Customs in this PR, or in a future one?

@slava77
Copy link
Contributor

slava77 commented Nov 2, 2015

@richard-cms I think this function can be included in customise_Reco:
https://github.com/cms-sw/cmssw/blob/CMSSW_7_5_4/SLHCUpgradeSimulations/Configuration/python/postLS1Customs.py#L761

If you propagate this PR to 76X, this modification of customise_Reco should not be propagated.

…Reco() in file SLHCUpgradeSimulations/Configuration/python/postLS1Customs.py
@ttrk
Copy link
Contributor Author

ttrk commented Nov 3, 2015

@slava77 I inserted the relevant function into the pointed position in SLHCUpgradeSimulations/Configuration/python/postLS1Customs.py .

@richard-cms

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 3, 2015

Pull request #12185 was updated. @cmsbuild, @cvuosalo, @civanch, @mdhildreth, @slava77 can you please check and sign again.

@slava77
Copy link
Contributor

slava77 commented Nov 3, 2015

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 3, 2015

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/9428/console

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 3, 2015

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 3, 2015

@civanch
Copy link
Contributor

civanch commented Nov 3, 2015

+1

@slava77
Copy link
Contributor

slava77 commented Nov 4, 2015

... testing ...

@slava77
Copy link
Contributor

slava77 commented Nov 4, 2015

+1

for #12185 cef7230

  • code looks ok
  • jenkins tests pass and comparisons with baseline show no differences
  • new modules appear in run2 data and MC processing and take under 40ms/event based on 134.802 (2015C DoubleEG)
       added      +0.16%         0.00 ms/ev ->        17.81 ms/ev photonIsolationHIProducerppGED
       added      +0.11%         0.00 ms/ev ->        12.08 ms/ev photonIsolationHIProducerpp
       added      +0.05%         0.00 ms/ev ->         5.18 ms/ev islandBasicClusters
  • new products appear in pp run2 RECO: increasing the size by ~0.5%
    • recoHIPhotonIsolationedmValueMap_photonIsolationHIProducerpp
    • recoHIPhotonIsolationedmValueMap_photonIsolationHIProducerppGED
    • recoCaloClusters_islandBasicClusters_islandBarrelBasicClusters
    • recoCaloClusters_islandBasicClusters_islandEndcapBasicClusters
  • new products appear in pp run2 AOD: increasing the size by ~0.2%
    • recoHIPhotonIsolationedmValueMap_photonIsolationHIProducerpp
    • recoHIPhotonIsolationedmValueMap_photonIsolationHIProducerppGED

@slava77
Copy link
Contributor

slava77 commented Nov 4, 2015

@ttrk
please make a 76X PR with only

  • RecoHI/Configuration/python/customise_PPwithHI.py
  • RecoHI/HiEgammaAlgos/python/photonIsolationHIProducer_cfi.py
    (no modifications to SLHCUpgradeSimulations/Configuration/python/postLS1Customs.py )

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 4, 2015

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_5_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_7_6_X is complete. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar

@davidlange6
Copy link
Contributor

+1

cmsbuild added a commit that referenced this pull request Nov 5, 2015
pp RECO with HI photon isolation using "customise" option
@cmsbuild cmsbuild merged commit 4df4154 into cms-sw:CMSSW_7_5_X Nov 5, 2015
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