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

make pileup-JME default for top projection in PFBRECO sequence #3535

Closed

Conversation

monttj
Copy link
Contributor

@monttj monttj commented Apr 27, 2014

No description provided.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @monttj (Tae Jeong Kim) for CMSSW_7_1_X.

make pileup-JME default for top projection in PFBRECO sequence

It involves the following packages:

CommonTools/ParticleFlow

@nclopezo, @monttj, @cmsbuild, @anton-a, @thspeer, @slava77, @vadler, @Degano can you please review it and eventually sign? Thanks.
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.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@monttj
Copy link
Contributor Author

monttj commented Apr 27, 2014

make JME way to identify pileup default by turning off following option in default sequence
checkClosestZVertex = cms.bool(False)

  • not include charged hadrons as pileup if there is no vertex assoicated

@ferencek
Copy link
Contributor

@monttj I don't think this is sufficient. If you load

PhysicsTools/PatAlgos/test/patTuple_PF2PAT_cfg.py

and backtrack how

process.patJetsPFlow

are produced you will see that they still come from the 'JME' instance of pfNoPU.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@monttj
Copy link
Contributor Author

monttj commented Apr 28, 2014

okay, it has been changed all downstream of PFBRECO.
Since it shares with reco sequence (I believe so), we can stick to "JME" postfix for now not to make further confusion.

@monttj
Copy link
Contributor Author

monttj commented Apr 28, 2014

+1

@ferencek
Copy link
Contributor

@monttj since #3510 has been merged, you can close this PR.

@nclopezo nclopezo added this to the CMSSW_7_1_0_pre8 milestone Apr 29, 2014
@nclopezo nclopezo removed this from the CMSSW_7_1_0_pre7 milestone Apr 29, 2014
@slava77
Copy link
Contributor

slava77 commented Apr 30, 2014

Hi Taejeong

Should I expect any changes in RECO objects or in the DQM plots?
If so, some notes would help

@ferencek
Copy link
Contributor

@slava77 This PR can be closed. This was an attempt to modify some PFBRECO modules so that example PAT config files stay backward compatible. But as Taejeong pointed out, PFBRECO sequence is now also used in the standard reconstruction so it is easier to just update the PAT config files which was done in #3510

@slava77
Copy link
Contributor

slava77 commented Apr 30, 2014

-1

Hi Dinko,

Thanks for the comment.
I'm giving a -1 to take off the RECO list and Taejeong can then close it.

@monttj
Copy link
Contributor Author

monttj commented May 6, 2014

Hi Slava,

Sorry for being late.
I wanted to remove the postfix "JME" in PAT configuration.

It does not affect the reco with changes as reco uses "JME" which this issue was trying to make as default without postfix.

Taejeong

@slava77
Copy link
Contributor

slava77 commented May 6, 2014

Hi Taejeong,

I'm confused.
Should this PR be reopened and considered for the release
or did you just provide more details to the comments from Dinko and this PR being closed is OK (because there is a different solution elsewhere)

Please clarify.
(if needs to be reopened, please reopen)

@monttj
Copy link
Contributor Author

monttj commented May 6, 2014

Hi Slava,

I tried to provide more details in addition to the comments from Dinko.
It does not need to be reopened for now.

Taejeong

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