vallot / CATTools

for cms analysis
4 stars 45 forks source link

Possible bug in GenTop.cc #79

Closed jhgoh closed 9 years ago

jhgoh commented 9 years ago

(Already known though mailing list, but posting here for tracking)

https://github.com/vallot/CATTools/blob/cat72x/DataFormats/src/GenTop.cc#L426

 vector<const reco::Candidate*>::iterator it = find ( mapJetToBHadrons[idx].begin(), mapJetToBHadrons[idx].begin(), lastB );
if( it == mapJetToBHadrons[idx].end() ){
mapJetToBHadrons[idx].push_back(lastB);
mapJetToBMatched[idx] = 1;
mapBHadronToJets[lastB].push_back( idx );
}

Finds 'lastB' from begin() to begin(), seem to be a typo. If it is intended, simply mapJetToBHadrons[idx].begin() == lastB is enough.

jshlee commented 9 years ago

@YoungKwonJo do you know whats going on with this? Is anyone planing to validated this with new MC or pruned genParticles?

YoungKwonJo commented 9 years ago

https://github.com/vallot/CATTools/blob/cat72x/DataFormats/src/GenTop.cc#L426 vector<const reco::Candidate*>::iterator it = find ( mapJetToBHadrons[idx].begin(), mapJetToBHadrons[idx].end(), lastB );

https://github.com/vallot/CATTools/blob/cat72x/DataFormats/src/GenTop.cc#L436 vector<const reco::Candidate*>::iterator it = find ( mapJetToCHadrons[idx].begin(), mapJetToCHadrons[idx].end(), lastC );

If we change them as above, It is looks O.K. However for the detail, tjkim can say that.

And for the validation jobs, if we make the catTuple, we will trying to check the signal definition(ttbb,ttbj,ttcc,ttLF) by using gentop

jhgoh commented 9 years ago

Yes, I was just waiting for @monttj 's answer.

YoungKwonJo commented 9 years ago

In GenTop, This is for genParticle part. https://github.com/vallot/CATTools/blob/cat72x/DataFormats/src/GenTop.cc#L61 This is for genJet part. https://github.com/vallot/CATTools/blob/cat72x/DataFormats/src/GenTop.cc#L366 We can see the genJet with b/c hardron in matching. However (I guess) I couldn't found code about matching between bquark of genparticle and genJet.

I find a presentation at (Thursday, 21 August 2014) https://indico.cern.ch/event/331035/contribution/5/material/slides/0.pdf see page 16 in the slide. We need to match between b and b-hadron in the slide. (old one: https://indico.cern.ch/event/271460/session/1/contribution/4/material/slides/0.pdf)

This is the related-pull in cmssw. https://github.com/cms-sw/cmssw/pull/4934 and source code for detail : https://github.com/cms-sw/cmssw/blob/CMSSW_7_5_X/PhysicsTools/JetMCAlgos/plugins/GenHFHadronMatcher.cc and test code : https://github.com/cms-sw/cmssw/blob/CMSSW_7_5_X/PhysicsTools/JetMCAlgos/test/matchGenHFHadrons.py

I guess the code is already in general cmssw.

Can we use the option in genJet in the code as https://github.com/vallot/CATTools/blob/v7-1-1/DataFormats/interface/GenJet.h ?

What do you think about it?

jshlee commented 9 years ago

currently, catJet has hadron and parton flavour matching done https://github.com/vallot/CATTools/blob/cat72x/DataFormats/interface/Jet.h#L39-L40

plus a link to it's genjet https://github.com/vallot/CATTools/blob/cat72x/DataFormats/interface/Jet.h#L75

I don't quite follow your proposal. Also, it doesn't fix issue.

YoungKwonJo commented 9 years ago

I mean about genJet, not Jet(Reco). Because the genJet with matching b or b-hadron is very important for ttbb signal definition.

Perhaps, I would like to find a easy way for ttbb signal definition without genTop or with updated genTop.

jshlee commented 9 years ago

yep but currently only slimmedGenJets is saved because catJets have links to them you can covert these into catGenJets and relink the catJets to the new collection

jhgoh commented 9 years ago

@YoungKwonJo, is it a request for new features? Still, the question remains as unanswered. Question : is the begin() to begin() intended one or a bug? To finalize this issue, I'll submit new PR.

YoungKwonJo commented 9 years ago

Yes, it is for new features.

YoungKwonJo commented 9 years ago

1. I would like to change my comment in previous comments.

However (I guess) I couldn't found code about matching between bquark of genparticle and genJet.

I find the code for matching between bquark of genparticle and genJet. https://github.com/vallot/CATTools/blob/cat72x/DataFormats/src/GenTop.cc#L380

2. And, I would like to change the genJetLabel from "ak4GenJets" to "ak4GenJetsNoNu". https://github.com/vallot/CATTools/blob/cat72x/CatProducer/python/genTopProducer_cfi.py#L5

cf. https://github.com/cms-sw/cmssw/blob/CMSSW_7_5_X/RecoJets/Configuration/python/RecoGenJets_cff.py

3. However, the new features can be upgraded our-result by following the slide. https://indico.cern.ch/event/331035/contribution/5/material/slides/0.pdf

jhgoh commented 9 years ago

Changing the GenTop is up to the people involved in ttbb analysis. Adding new feature like genParticle-genJet association is simply an addition, but changing the input collection to *GenJetsNoNu will introduce physics change. It must be discussed during the meeting. I think there is freedom to to include neutrinos or not in your physics analysis, but there must be some common selections. Using genJets without neutrinos can be thought to be reasonable, but anyway this will distort your "truth" distribution and introduce additional differences between genJet vs b quarks. Comparison plot between genJets with and without neutrinos can be helpful.

YoungKwonJo commented 9 years ago

If we think visible phase space selection, (I guess) using GenJetNoNu can be good option.

YoungKwonJo commented 9 years ago

new feature for GenTop, http://cms.cern.ch/iCMS/jsp/db_notes/noteInfo.jsp?cmsnoteid=CMS%20AN-2014/093

jhgoh commented 9 years ago

@YoungKwonJo There's new PR in main CMSSW to switch genJet without neutrinos. https://github.com/cms-sw/cmssw/pull/8214 which advocates your opinion.

jshlee commented 9 years ago

looks like a string search and replace in all cmssw would have been better to redefine genJets as genJetsNoNu

jhgoh commented 9 years ago

This issue somehow moved to different direction. Anyway, I'm closing this issue.