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

Tau reconstruction based on miniAOD event content #22594

Merged
merged 129 commits into from
Apr 6, 2019

Conversation

roger-wolf
Copy link
Contributor

Hi all,

this is a pull request concerned with tau reconstruction based on miniAOD event content. There has been a related PR (*) in December. After some restructuring, following proposals by Slava, this is the next iteration to integrate this feature. The restructuring required changes of data formats to be a accepted by plugins and other data formats used in TauReco, which imply small/minimal changes in quite a large number of files. The original work was initiated by @mbluj and continued and brought to completion by @steggema. An independent deep validation has been performed by @mbluj in addition to the usual checks that we apply. All tests went out fine.

We have checked and expect that the default workflows based on AOD input remain unchanged. Small differences are observed and expected when comparing taus reconstructed based on AOD input to taus reconstructed based on miniAOD input. Also for the availability of the full performance of the tau-ID (e.g. concerning discrimination against muons) a few pieces of information are still missing on the miniAOD event content. This work is left to follow-up implementations.

A presentation by @steggema is anticipated in then next Reco/AT meeting. For more details before this presentation will be given, please refer to @steggema and @mbluj.

Cheers,
Roger

(*)
#21757

steggema and others added 30 commits December 18, 2017 13:15
…te to Candidate and PFJet to BaseJet in PFTau
…sed on work by Michal Bluj), and adapt MiniAOD tau sequence accordingly
…sed on work by Michal Bluj), and adapt MiniAOD tau sequence accordingly
…sed on work by Michal Bluj), and adapt MiniAOD tau sequence accordingly
@fabiocos
Copy link
Contributor

fabiocos commented Apr 4, 2019

@Martin-Grunewald @alja @santocch @andrius-k could you please check this PR?

@alja
Copy link
Contributor

alja commented Apr 4, 2019

+1

@andrius-k
Copy link

+1

@Martin-Grunewald
Copy link
Contributor

+1

@fabiocos
Copy link
Contributor

fabiocos commented Apr 5, 2019

@santocch the analysis-only part is restricted to two files, could you please check this PR? I would like to finally move it forward asap

@fabiocos
Copy link
Contributor

fabiocos commented Apr 6, 2019

+1

this PR is plenty of merges, which is not the ideal way of updating, but from integration tests it dos not seem to involve any previously merged PR, and the history looks preserved in a reasonable way

@fabiocos
Copy link
Contributor

fabiocos commented Apr 6, 2019

merge

@santocch this is blocking other fixes, I move forward with the integration, the PhysicsTools look reasonable to me. Please check it in detail and sign it or propose further updates to be followed in a separate PR

@cmsbuild cmsbuild merged commit 18281d6 into cms-sw:master Apr 6, 2019
@peruzzim
Copy link
Contributor

peruzzim commented Apr 7, 2019

We would like to understand whether this functionality could - technically - be backported to 102X, or whether it requires other changes that are not present there.
This, to understand the boundary conditions in view of a possible future re-miniAOD workflow from miniAOD. We are not requesting this backport to take place now.

const reco::PFCandidatePtr& pfCand = isolationPFCands.at(iPFCand);
std::cout << " pfCand #" << iPFCand << " (" << pfCand.id() << ":" << pfCand.key() << "):"
<< " Pt = " << pfCand->pt() << ", eta = " << pfCand->eta() << ", phi = " << pfCand->phi() << std::endl;
const auto& isolationCands = tauRef_->isolationCands();
Copy link
Contributor

Choose a reason for hiding this comment

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

@perrotta @slava77 @steggema after merging this PR there is a list of massive failures in the clang build in this class:

Begin processing the 14th record. Run 1, Event 14, LumiSection 1 on stream 2 at 07-Apr-2019 10:29:06.605 CEST


A fatal system signal has occurred: segmentation violation
The following is the call stack containing the origin of the signal.

Sun Apr  7 10:29:06 CEST 2019

Current Modules:

Module: RecoTauCleaner:hpsPFTauProducerSansRefs (crashed)
Module: PoolOutputModule:AODSIMoutput
Module: RPCDigiProducer:simMuonRPCDigis
Module: FastSimProducer:fastSimProducer

A fatal system signal has occurred: segmentation violation

@santocch
Copy link

santocch commented Apr 7, 2019

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 7, 2019

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will be automatically merged.

@Dr15Jones
Copy link
Contributor

I was able to reproduce the crash under clang and got a traceback

#4  <signal handler called>
#5  __cxxabiv1::__dynamic_cast (src_ptr=0x0, src_type=0x7fab20dd6738 <typeinfo for reco::Candidate>, dst_type=0x7fab27d9d590 <typeinfo for pat::PackedCandidate>, src2dst=0) at ../../../../libstdc++-v3/libsupc++/dyncast.cc:50
#6  0x00007faac8872d5f in reco::tau::getTrackFromChargedHadron(reco::PFRecoTauChargedHadron const&) () from /cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc700/cms/cmssw/CMSSW_10_6_CLANG_X_2019-04-07-0000/lib/slc7_amd64_gcc700/libRecoTauTagRe
coTau.so
#7  0x00007faac8cc2bb8 in reco::tau::RecoTauSoftTwoProngTausCleanerPlugin::operator()(edm::Ref<std::vector<reco::PFTau, std::allocator<reco::PFTau> >, reco::PFTau, edm::refhelper::FindUsingAdvance<std::vector<reco::PFTau, std::allocator
<reco::PFTau> >, reco::PFTau> > const&) const () from /cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc700/cms/cmssw/CMSSW_10_6_CLANG_X_2019-04-07-0000/lib/slc7_amd64_gcc700/pluginRecoTauTagRecoTauPlugins.so
#8  0x00007faac8bd9cbf in RecoTauCleanerImpl<std::vector<reco::PFTau, std::allocator<reco::PFTau> > >::produce(edm::Event&, edm::EventSetup const&) () from /cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc700/cms/cmssw/CMSSW_10_6_CLANG_X_2019-
04-07-0000/lib/slc7_amd64_gcc700/pluginRecoTauTagRecoTauPlugins.so

@Dr15Jones
Copy link
Contributor

The fix in in #26381

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.