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

Add PFCluster Isolation to miniAOD electrons and photons #8221

Closed

Conversation

matteosan1
Copy link
Contributor

As the title says this PR add to floats to electron and photon to store PFCluster Isolation as computed at the HLT.
It contains:

  • Factorized isolation computation algorithm so that it can be run by the HLT and offline producers (RecoEgamma/EgammaIsolationAlgos). I have checked that HLT results stays the same.
  • Modified Electron.h and Photon.h DF to add the two numbers. (DataFormats/PatCandidates)
  • Updated PAT and MiniAOD configurations to store the value in the object. (PhysicsTools/PatAlgos)

The idea is to compute at RECO level the valuemap with the isolation values and propagate it to the miniAOD.
Right now the filling is not activated since we are missing the ValueMap into RECO relvals so it wouldn't pass the standard tests. I have privately checked the filling is ok.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @matteosan1 (Matteo Sani) for CMSSW_7_4_X.

Add PFCluster Isolation to miniAOD electrons and photons

It involves the following packages:

DataFormats/PatCandidates
PhysicsTools/PatAlgos
RecoEgamma/Configuration
RecoEgamma/EgammaHLTProducers
RecoEgamma/EgammaIsolationAlgos
RecoEgamma/EgammaPhotonProducers

@perrotta, @cmsbuild, @nclopezo, @cvuosalo, @fwyzard, @monttj, @Martin-Grunewald, @slava77, @vadler can you please review it and eventually sign? Thanks.
@rappoccio, @Sam-Harper, @imarches, @ahinzmann, @acaudron, @mmarionncern, @lgray, @jdolen, @nhanvtran, @schoef, @ferencek, @mariadalfonso, @pvmulder, @TaiSakuma 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' 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.

@@ -18,7 +18,7 @@
<!-- PAT Objects, and embedded data -->
<class name="pat::Electron" ClassVersion="29">
<version ClassVersion="29" checksum="1784986402"/>
<version ClassVersion="28" checksum="2518240031"/>
<version ClassVersion="28" checksum="1881133053"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't you have added a ClassVersion 30 instead of modifying the 28?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it was 27 (but version 28 was listed below anyway) and it told
me to add 29.
I don't know why it changed the checksum for 28.

On 12.03.2015 13:22, Giovanni Petrucciani wrote:

In DataFormats/PatCandidates/src/classes_def_objects.xml
#8221 (comment):

@@ -18,7 +18,7 @@

- -

Shouldn't you have added a |ClassVersion| 30 instead of modifying the 28?


Reply to this email directly or view it on GitHub
https://github.com/cms-sw/cmssw/pull/8221/files#r26296917.

Matteo Sani
University of California, San Diego
+41227671577

Copy link
Contributor

Choose a reason for hiding this comment

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

Another pull request could have updated the same file

Copy link
Contributor

Choose a reason for hiding this comment

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

Needs to be fixed as it does not compile:

error: class 'pat::Electron' has a different checksum for ClassVersion 29. Increment ClassVersion to 30 and assign it to checksum 3508125821
Suggestion: You can run 'scram build updateclassversion' to generate src/DataFormats/PatCandidates/src/classes_def_objects.xml.generated with updated ClassVersion

@Martin-Grunewald
Copy link
Contributor

Need a PR for 75X as well?

@Martin-Grunewald
Copy link
Contributor

+1

@matteosan1
Copy link
Contributor Author

Yes, I am going to do it today.

On 13.03.2015 12:04, Martin Grunewald wrote:

Need a PR for 75X as well?


Reply to this email directly or view it on GitHub
#8221 (comment).

Matteo Sani
University of California, San Diego
+41227671577

@slava77
Copy link
Contributor

slava77 commented Mar 24, 2015

-1
replaced by #8501

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