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

Isolation CITK 80X (rebased) #13391

Merged

Conversation

ishvetso
Copy link
Contributor

update for CITK isolation module in 76X: included puppi-based isolation, map based veto (electron and photon) and cone veto (photons)

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @ishvetso (Ivan Shvetsov) for CMSSW_8_0_X.

It involves the following packages:

PhysicsTools/IsolationAlgos
RecoEgamma/EgammaIsolationAlgos

@cvuosalo, @monttj, @cmsbuild, @slava77, @vadler, @davidlange6 can you please review it and eventually sign? Thanks.
@Sam-Harper, @lgray this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@ishvetso ishvetso mentioned this pull request Feb 18, 2016
@slava77
Copy link
Contributor

slava77 commented Feb 18, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

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

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@@ -6,5 +6,6 @@
<use name="DataFormats/PatCandidates"/>
<use name="DataFormats/TauReco"/>
<use name="DataFormats/TrackReco"/>
<use name="DataFormats/PatCandidates"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove this line. It is a duplicate in 80X version.

@cmsbuild
Copy link
Contributor

Pull request #13391 was updated. @cvuosalo, @monttj, @cmsbuild, @slava77, @vadler, @davidlange6 can you please check and sign again.

@slava77
Copy link
Contributor

slava77 commented Feb 18, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

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

@cmsbuild
Copy link
Contributor

-1
Tested at: b45d01a
When I ran the RelVals I found an error in the following worklfows:
5.1 step1

runTheMatrix-results/5.1_TTbar+TTbarFS+HARVESTFS/step1_TTbar+TTbarFS+HARVESTFS.log

135.4 step1

runTheMatrix-results/135.4_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS/step1_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS.log

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13391/11351/summary.html

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
4962138
67ba1c8
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13391/11351/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13391/11351/git-merge-result

@slava77
Copy link
Contributor

slava77 commented Feb 18, 2016

another (less) broken IB, sigh.

@slava77
Copy link
Contributor

slava77 commented Feb 18, 2016

+1

for #13391 b45d01a

  • code changes are OK, the same as in 74X version. All new code for use in analysis.
  • jenkins tests pass where needed (compilation and code quality checks)

@davidlange6
Copy link
Contributor

this time from code that passed all checks.. backing it out

On Feb 18, 2016, at 9:47 PM, Slava Krutelyov notifications@github.com wrote:

another (less) broken IB, sigh.


Reply to this email directly or view it on GitHub.

@ishvetso
Copy link
Contributor Author

This branch contains modules to run isolation sums for electrons and photons: with map based veto and cone based veto, all these types of isolations can done for done for PUPPI-based isolation.

Modules can run either on miniAOD or AOD: for miniAOD one can use PUPPI weights that are stored in the packedCandidate directly by using CITKPFIsolationSumProducerForPUPPI with empty string for puppiValueMap. One can also rerun PUPPI and use PUPPI collection as input: for cone-based veto one should specify name of PUPPI collection and can use CITKPFIsolationSumProducer, for map-based veto one should specify the name of PUPPI value map (which is the same name as the name of PUPPI collection) but the name of collection should be either packedPFCandidates or pfNoPileupCanidates depending whether one runs on miniAOD or AOD (this is also illustrated in example in python/). For CITK twiki please refer here:

https://twiki.cern.ch/twiki/bin/viewauth/CMS/CommonIDAndIsolationFW

Studies have been for PUPPI-based isolation have been presented here:

https://indico.cern.ch/event/494563/contribution/2/attachments/1225182/1793161/iPUPPI_photons_EGM_9February.pdf

https://indico.cern.ch/event/482699/contribution/2/attachments/1215812/1775490/Shvetsov_update_iPUPPI_electrons.pdf

Electron cone based veto is not touched in this PR: I will create another PR to update as it does follow pfIsolationVariables 100%.

davidlange6 added a commit that referenced this pull request Mar 8, 2016
@davidlange6 davidlange6 merged commit b9bddde into cms-sw:CMSSW_8_0_X Mar 8, 2016
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

4 participants