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

crash in MergedGenParticleProducer #82

Closed
arizzi opened this issue Nov 27, 2017 · 9 comments
Closed

crash in MergedGenParticleProducer #82

arizzi opened this issue Nov 27, 2017 · 9 comments

Comments

@arizzi
Copy link

arizzi commented Nov 27, 2017

we have a crash here:

#5 0x00007f6d6c89183a in MergedGenParticleProducer::isPhotonFromPrunedHadron(pat::PackedGenParticle const&) const () from /cvmfs/cms.cern.ch/slc6_amd64_gcc630/cms/cmssw/CMSSW_9_4_0/lib/slc6_amd64_gcc630/pluginGeneratorInterfaceRivetInterface_plugins.so

you can reproduce with (on lxplus) ~arizzi/public/test92X_NANO.py on 940 release + nanoaod master branch

@intrepid42

@arizzi
Copy link
Author

arizzi commented Nov 27, 2017

it looks like the sample has a fancy particle listing with only a few particles and perhaps some assumptions of the GenParticles2HepMCConverter do not hold. @perrozzi

@mseidel42
Copy link

I can confirm, fixing the problem in MergedGenParticleProducer (see below) leads to a crash in this line of GenParticles2HepMCConverter:

  HepMC::GenVertex* vertex1 = new HepMC::GenVertex(FourVector(parton1->vertex()));

Using John's latest patch (cms-sw#21349) does not solve the problem. Does this sample even have incoming protons stored??

@jhgoh Do you have any idea? This is the input file: root://cmsxrootd.hep.wisc.edu//store/mc/RunIISummer17MiniAOD/GGToEE_Pt-50_Elastic_13TeV-lpair/MINIAODSIM/92X_upgrade2017_realistic_v10-v1/100000/5A440CEE-DCA5-E711-9424-0242AC11001A.root

@jhgoh
Copy link

jhgoh commented Nov 27, 2017

Yes I can reproduce crash even after applying PR21349.
I was asking a parton to have |eta| > 100 (just a random large number), but in this generator, this selection was too tight to catch them.

# pdgId eta pz
22 -6.86314 -103.904
22 7.69135 31.1981
2212 -10.9831 -6396.1
-11 -0.100614 -5.08589
11 -1.42446 -98.8182
2212 13.0257 6468.8
11 -1.10236 -67.6201

I guess releasing eta cut will fix the problem. let me try.

@jhgoh
Copy link

jhgoh commented Nov 27, 2017

OK, two separate problems as pointed by @arizzi

  1. missing guard in MergedGenParticleProducer
diff --git a/GeneratorInterface/RivetInterface/plugins/MergedGenParticleProducer.cc b/GeneratorInterface/RivetInterface/plugins/MergedGenParticleProducer.cc
index 42cf610af0e..895e93465f4 100644
--- a/GeneratorInterface/RivetInterface/plugins/MergedGenParticleProducer.cc
+++ b/GeneratorInterface/RivetInterface/plugins/MergedGenParticleProducer.cc
@@ -133,6 +133,7 @@ void MergedGenParticleProducer::produce(edm::Event& event, const edm::EventSetup
 
 bool MergedGenParticleProducer::isPhotonFromPrunedHadron(const pat::PackedGenParticle& pk) const
 {
+  if ( pk.mother(0) == nullptr ) return false;
   HepPDT::ParticleID motherid(pk.mother(0)->pdgId());
   return
     ( pk.pdgId() == 22 // We care about photons for lepton dressing here
  1. Tight eta cut on incident particle.
    set_beam_particles() had to be commented out to avoid "Event beams mismatch"
diff --git a/GeneratorInterface/RivetInterface/plugins/GenParticles2HepMCConverter.cc b/GeneratorInterface/RivetInterface/plugins/GenParticles2HepMCConverter.cc
index c7fe72ef4c8..d65a9f9a3f0 100644
--- a/GeneratorInterface/RivetInterface/plugins/GenParticles2HepMCConverter.cc
+++ b/GeneratorInterface/RivetInterface/plugins/GenParticles2HepMCConverter.cc
@@ -113,7 +113,7 @@ void GenParticles2HepMCConverter::produce(edm::Event& event, const edm::EventSet
     genCandToHepMCMap[p] = hepmc_particle;
 
     // Find incident proton pair
-    if ( p->pdgId() == 2212 and std::abs(p->eta()) > 100 and std::abs(p->pz()) > 1000 ) {
+    if ( p->mother(0) == nullptr and std::abs(p->eta()) > 5 and std::abs(p->pz()) > 1000 ) {
       if ( !parton1 and p->pz() > 0 ) {
         parton1 = p;
         hepmc_parton1 = hepmc_particle;
@@ -132,7 +132,7 @@ void GenParticles2HepMCConverter::produce(edm::Event& event, const edm::EventSet
   hepmc_event->add_vertex(vertex2);
   vertex1->add_particle_in(hepmc_parton1);
   vertex2->add_particle_in(hepmc_parton2);
-  hepmc_event->set_beam_particles(hepmc_parton1, hepmc_parton2);
+  //hepmc_event->set_beam_particles(hepmc_parton1, hepmc_parton2);

Since the PR is not closed yet, I can apply changes above if this is reasonable.

@mseidel42
Copy link

Hi John, thanks a lot!

+  if ( pk.mother(0) == nullptr ) return false;

I think this will miss out on orphaned photons where no mother information is stored. Please see mseidel42@b49a38c for my solution.

@arizzi
Copy link
Author

arizzi commented Nov 27, 2017

can you guys push the fixes in cms-sw/cmssw both for master and 94X branches?

@jhgoh
Copy link

jhgoh commented Nov 27, 2017

@intrepid42 I can cherry-pick this one, if it is not already in the master branch.
@arizzi I'ml pushing the change now (at least for the 2nd point to GenParticles2HepMCConverter)

@mseidel42
Copy link

I can cherry-pick this one, if it is not already in the master branch.

Yes, please, I made the patch this morning :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants