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

MiniAOD b-tag tweaks #8370

Conversation

ferencek
Copy link
Contributor

This PR introduces the following changes:

  • adds b-tag discriminators to AK8 jets (the set of discriminators kept in sync with the PAT jet defaults)
  • updates subjet b-tag discriminators from CSV+AVR to CSVv2+AVR
  • removes b tagging from CMSTopTag fat jets (no need to run it since these jets are not stored in MiniAOD anyways)
  • puts SoftDrop subjets in front of CMSTopTag subjets (this is important to correctly store the AK8 jet daughters since the CMSTopTag subjets are derived from CA8 jets and could in principle contains extra constituents not clustered inside AK8 jets. SoftDrop subjets are derived from AK8 jets).
  • fixes minor inconsistencies in TopTagInfo handling

Observed increase in the event size is at sub-per-mille level (tested on 9k events from /RelValProdTTbar_13/CMSSW_7_4_0_pre8-MCRUN2_74_V7-v1/AODSIM).

@gpetruc @arizzi @rappoccio @dmajumder

To correctly store daughters of AK8 jets it is important to use subjets
actually derived from AK8 jets. In this case these are the SoftDrop subjets
while the CMSTopTag subjets are derived from CA8 jets and could in
principle contains extra constituents not clustered inside AK8 jets.
@arizzi
Copy link
Contributor

arizzi commented Mar 18, 2015

@ferencek is there a twin PR on 75X?

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @ferencek (Dinko Ferencek) for CMSSW_7_4_X.

MiniAOD b-tag tweaks

It involves the following packages:

PhysicsTools/PatAlgos

@cmsbuild, @vadler, @nclopezo, @monttj can you please review it and eventually sign? Thanks.
@rappoccio, @imarches, @ahinzmann, @acaudron, @mmarionncern, @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.

@ferencek
Copy link
Contributor Author

@arizzi, I first wanted this to be reviewed and tested for 74X. Also, the policy on automatic vs manual forward porting is a bit confusing so I did not want to rush with a 75X PR. If and when needed, a 75X PR will be created.

@ferencek
Copy link
Contributor Author

Just a general comment. The way we handle AK8 daughters at the moment is a but fragile. If somebody in the future reverses the order of subjet collections or adds a new one in front, things might not work as intended. Should there be a comment left in the cfg file?

@rappoccio
Copy link
Contributor

@ferencek yes, that would definitely help.

@gpetruc
Copy link
Contributor

gpetruc commented Mar 18, 2015

Ah, ok, now I understand... indeed we assume that the subjets only contain
candidates from the original jets.

A protection could be to change these lines

https://github.com/cms-sw/cmssw/blob/CMSSW_7_4_X/PhysicsTools/PatUtils/plugins/JetSubstructurePacker.cc#L77-L78
so that if a subjet has a daughter that is not also daughter of the AK8
then the subjet is not added as daughter and its daughters are not marked
as already added.

Giovanni

On Wed, Mar 18, 2015 at 7:00 PM, rappoccio notifications@github.com wrote:

@ferencek https://github.com/ferencek yes, that would definitely help.


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

@ferencek
Copy link
Contributor Author

@gpetruc, this is in principle OK from the point of view of ensuring that AK8 daughters are all stored correctly but this is not necessarily optimal from the event size point of view. CA8 and AK8 jets generally tend to have quite different shapes so I wouldn't be surprised that subjets derived from CA8 jets quite often contain some extra constituents wrt to AK8 jets. So a possible (and maybe not so unlikely) scenario is that because of a single extra constituent in one or more of the subjets, we would duplicate a lot of daughter pointers.

@rappoccio
Copy link
Contributor

@ferencek Even the CA8 jets are preclustered with AK8, though, so I'm not sure this happens in practice. You may be right but I think these corner cases are pretty rare.

@ferencek
Copy link
Contributor Author

@rappoccio, aah, indeed, preclustering would reduce the likelihood of this happening (I somehow keep forgetting about preclustering ;) ). However, in busy final states, where it maybe matters even more not to duplicate stuff, this would probably not be so unlikely.

@ferencek ferencek force-pushed the MiniAODBTagTweaks_from-CMSSW_7_4_X_2015-03-17-1400 branch from abfb5c7 to 929961c Compare March 18, 2015 18:42
@gpetruc
Copy link
Contributor

gpetruc commented Mar 18, 2015 via email

@ferencek
Copy link
Contributor Author

OK, I added a comment but it would probably still be good to implement Giovanni's suggestion.

@cmsbuild
Copy link
Contributor

Pull request #8370 was updated. @cmsbuild, @vadler, @nclopezo, @monttj can you please check and sign again.

@ferencek ferencek force-pushed the MiniAODBTagTweaks_from-CMSSW_7_4_X_2015-03-17-1400 branch from 8d6ce63 to 2a05498 Compare March 18, 2015 20:40
@ferencek
Copy link
Contributor Author

@gpetruc, please have a look at the latest commit and let me know if there is anything missing or can be improved.

@cmsbuild
Copy link
Contributor

Pull request #8370 was updated. @cmsbuild, @vadler, @nclopezo, @monttj can you please check and sign again.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@gpetruc
Copy link
Contributor

gpetruc commented Mar 19, 2015

@ferencek the modification looks ok to me

davidlange6 added a commit that referenced this pull request Mar 21, 2015
…W_7_4_X_2015-03-17-1400

MiniAOD b-tag tweaks
@davidlange6 davidlange6 merged commit e110593 into cms-sw:CMSSW_7_4_X Mar 21, 2015
@ferencek ferencek deleted the MiniAODBTagTweaks_from-CMSSW_7_4_X_2015-03-17-1400 branch March 21, 2015 16:19
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