Make jets in TauGenJetProducer from taus which are not decayed #41064

Dr15Jones · 2023-03-15T19:52:51Z

PR description:

Instead of creating a dummy empty jet, taus which were not decayed by the generator are now used to create a jet based on the tau's own data. This avoids failures in the integration builds.

Did some minor code cleanups.

PR validation:

Code compiles.

Dr15Jones · 2023-03-15T19:53:22Z

Meant to fix #37169.

Dr15Jones · 2023-03-15T19:53:32Z

please test

cmsbuild · 2023-03-15T20:00:51Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41064/34665

This PR adds an extra 12KB to repository

cmsbuild · 2023-03-15T20:01:20Z

A new Pull Request was created by @Dr15Jones (Chris Jones) for master.

It involves the following packages:

PhysicsTools/JetMCAlgos (analysis)

can you please review it and eventually sign? Thanks.
@AlexDeMoor, @emilbols, @AnnikaStein, @JyothsnaKomaragiri, @andrzejnovak, @demuller this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

cmsbuild · 2023-03-15T22:18:45Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e98468/31300/summary.html
COMMIT: b5b3171
CMSSW: CMSSW_13_1_X_2023-03-15-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/41064/31300/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 6 lines to the logs
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3550756
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3550733
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 213 log files, 164 edm output root files, 49 DQM output files
TriggerResults: no differences found

perrotta · 2023-03-16T08:46:58Z

Thank you @Dr15Jones to provide a possible fix fur such a long standing issue.

The main issue seems to me reside in the generators which do not decay those taus: was it done on purpose (i.e. long lived)? In that case, an alternative could be to add the undecayed tau itself to the jet, i.e.

    if (descendents.empty()) {
      edm::LogWarning("NoTauDaughters") << "Tau p4: " << (*iTau)->p4() << " vtx: " << (*iTau)->vertex()
                                        << " has no daughters: the tau itself is added to the TauGenJet";
      charge += (*iTau)->charge();
      sumVisMom += (*iTau)->p4();
      constituents.push_back(refToPtr(*iTau));
      continue;
    }

If so, this should go after the check on the status 2 taus, and of course after the definition of constituents etc.

@cms-sw/tau-pog-l2 what do you think?

alebihan · 2023-03-16T11:19:24Z

Hi, I prefer to ask a few things before to greenlight this. I remember too that some generators make taus decay into themselves, and at the end of the cascade the final tau is decayed.
From what I see in cmssw TauGenJetsDecayModeSelectorAllHadrons_cfi is used in several places https://github.com/cms-sw/cmssw/search?q=TauGenJetsDecayModeSelectorAllHadrons_cfi, so one would need to make sure that there is no impact there. Would the idea be to add the tau itself to the collection of its constituents? But are we sure that the constituent lists is not filled at the end of the cascade?

perrotta · 2023-03-16T11:38:16Z

@alebihan in case your question refers to my proposed alternative to this PR:

The issue arises when the tau in NOT decayed: this is indeed a pathological case, and one should rather understand why does it happen, and try to deal with it
What proposed @Dr15Jones as fix is to ignore in that case those undecayed taus in the TauGenJetProducer, which is probably fine
What I suggest as a possible alternative is to put in that particular and pathological case in the TauGenJet itself instead of its missing decay products: I don't claim it is better than the previous one, but I would rather ask the tau POG experts about it
The fix applies only to the very few (hopefully) events that make wf 148.0 (and similar ones) crash because of the missing decay products

alebihan · 2023-03-16T13:12:54Z

Can you please share a recipe to see the crash? @mbluj @Ksavva1021 @danielwinterbottom what do you think?

mbluj · 2023-03-16T13:29:50Z

I think that is generally better to keep pathological entries than exclude them. However, it is good to add information which will allow easily identify them to enable its simple filtering or selection (depending on use-case). One can use for this purpose the status field of reco::GenJet. This field can be also used to store generated tau decay mode which I planned to add but forgot :( I can prepare shortly a followup PR adding status/decay mode for empty/correct genTauJets. Does it make sense?

Dr15Jones · 2023-03-16T14:51:17Z

@perrotta wrote

The issue arises when the tau in NOT decayed: this is indeed a pathological case, and one should rather understand why does it happen, and try to deal with it

So from the log of the failed job, the exception message showed that the taus have very large momentum such that the relativistic gamma factor times the half life of the tau gives a length greater than the distance to the beam pipe. That condition exceeds the threshold we've told Pythia to decay the taus (since the tau could interact with material and therefore should be handled by Geant). So I would not classify this as a pathological case, just a rare 'edge' case.

perrotta · 2023-03-16T15:00:39Z

@perrotta wrote

The issue arises when the tau in NOT decayed: this is indeed a pathological case, and one should rather understand why does it happen, and try to deal with it

So from the log of the failed job, the exception message showed that the taus have very large momentum such that the relativistic gamma factor times the half life of the tau gives a length greater than the distance to the beam pipe. That condition exceeds the threshold we've told Pythia to decay the taus (since the tau could interact with material and therefore should be handled by Geant). So I would not classify this as a pathological case, just a rare 'edge' case.

Ok, great!
Then this supports even more strongly the idea of propagating somehow the info about the generated tau to the TauGenJet; possibly with the additional flag suggested by @mbluj in #41064 (comment)

mbluj · 2023-03-16T15:01:59Z

@perrotta wrote

The issue arises when the tau in NOT decayed: this is indeed a pathological case, and one should rather understand why does it happen, and try to deal with it

So from the log of the failed job, the exception message showed that the taus have very large momentum such that the relativistic gamma factor times the half life of the tau gives a length greater than the distance to the beam pipe. That condition exceeds the threshold we've told Pythia to decay the taus (since the tau could interact with material and therefore should be handled by Geant). So I would not classify this as a pathological case, just a rare 'edge' case.

So, for me it makes even more sense to keep tauJets for such taus.

alebihan · 2023-03-16T15:33:45Z

So the tauGenJets would have a special flag to track them, contain the p4 of the undecayed tau (full p4 with neutrino), and the crash would be avoided. Makes sense, thanks.

mbluj · 2023-03-16T15:44:58Z

@Dr15Jones how we can proceed? Could you add genTauJets for the undecayed taus as proposed by @perrotta and then I will prepare a followup PR with status being either a code of tau-decay mode (a positive number as defined elsewhere for taus in CMS) or a "warning" flag, e.g. a negative number (are there better ideas?)?

Instead of creating a dummy empty jet, taus which were not decayed by the generator are now used to create a jet based on the tau's own data. This avoids failures in the integration builds.

cmsbuild · 2023-03-16T16:17:42Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41064/34684

This PR adds an extra 12KB to repository

cmsbuild · 2023-03-16T16:18:08Z

Pull request #41064 was updated. @cmsbuild can you please check and sign again.

Dr15Jones · 2023-03-16T16:18:23Z

please test

perrotta · 2023-03-16T16:44:21Z

PhysicsTools/JetMCAlgos/plugins/TauGenJetProducer.cc

@@ -41,6 +40,20 @@ void TauGenJetProducer::produce(edm::StreamID, Event& iEvent, const EventSetup&
    GenParticleRefVector descendents;
    findDescendents(*iTau, descendents, 1);

+    if (descendents.empty()) {


Uhm, shouldn't this go after the check for the status 2 taus in the descendents?
@alebihan @mbluj

(Maybe @mbluj could cherry pick this commit and take care of making a PR with the whole fix, so that he can include also the modified status all at once)

The taus causing the problem were never in the descendants, they always came from the main list.

cmsbuild · 2023-03-16T18:44:04Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e98468/31328/summary.html
COMMIT: bbd0d87
CMSSW: CMSSW_13_1_X_2023-03-16-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/41064/31328/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 9 lines from the logs
Reco comparison results: 12 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3550819
DQMHistoTests: Total failures: 11
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3550786
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 213 log files, 164 edm output root files, 49 DQM output files
TriggerResults: no differences found

perrotta · 2023-03-17T07:39:34Z

@alebihan @mbluj : please confirm that you are fine with this implementation, so that we can merge

alebihan · 2023-03-17T11:25:23Z

Looks good as far as I can see. So these taus are always HEPMC status 1 as not decayed on purpose?

mbluj · 2023-03-17T13:49:53Z

It is fine with me. I will follow with "status PR" when this one is merged.

perrotta · 2023-03-17T14:14:43Z

+1

Thank you @alebihan and @mbluj (and @Dr15Jones , of course)

perrotta · 2023-03-17T14:15:33Z

merge

perrotta · 2023-03-19T17:11:15Z

Even after the merge of this PR there are still instances of the error

----- Begin Fatal Exception 19-Mar-2023 10:32:47 CET-----------------------
An exception of category 'NoTauDaugters' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 1 stream: 1
   [1] Running path 'validation_step'
   [2] Calling method for module MBUEandQCDValidation/'mbueAndqcdValidation'
Exception Message:
 HepMCValidationHelper found no daughters for Tau within index 0 and info 
     20362       15 -6.41e+00,-3.57e+00,-1.26e+02,+1.26e+02   2 decay point ( did not decay )
  This should not be able to happen and needs to be fixed.
----- End Fatal Exception ----------------------------

See integration builds of CMSSW_13_1_X_2023-03-19-0000, namely wf 148.0 in el8_amd64_gcc11

mbluj · 2023-03-20T10:55:53Z

148.0 in

The issue is caused by this piece of code of HepMCValidationHelper (not owned by Tau POG): https://github.com/cms-sw/cmssw/blob/master/Validation/EventGenerator/src/HepMCValidationHelper.cc#L205-#L217.
The piece of code throws an exception when there is a genTau which does not decay. This code was updated about 2 weeks ago in a22109f to make the exception more verbose, namely to printout a position of decay vertex (if present), but the update does not change behavior of the exception. In principle one can try one of the following fixes:

Change the exception to LogError, which should be harmless for the rest of the code as far as I understand it;
Check if potential decay vertex of the tau is outside beam-pipe. As is such a case it is expected that taus are not decayed by a generator a LogWaring will be issued, otherwise an exception will be thrown;
Combination of the 1. & 2, i.e. LogWarning in case of a hypotetical decay outside beam-pipe and LogError otherwise.

I think that it will be best that this is decided and then implemented by owners of the code, but I can also do it if useful (I would opt for solution 2.).

cmsbuild added this to the CMSSW_13_1_X milestone Mar 15, 2023

cmsbuild added analysis-pending pending-signatures tests-pending orp-pending code-checks-pending labels Mar 15, 2023

cmsbuild added tests-started and removed tests-pending labels Mar 15, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 15, 2023

cmsbuild added tests-approved and removed tests-started labels Mar 15, 2023

Make jets from taus which are not decayed in TauGenJetProducer

bbd0d87

Instead of creating a dummy empty jet, taus which were not decayed by the generator are now used to create a jet based on the tau's own data. This avoids failures in the integration builds.

Dr15Jones changed the title ~~Ignore taus which are not decayed in TauGenJetProducer~~ Make jets from taus which are not decayed in TauGenJetProducer Mar 16, 2023

Dr15Jones force-pushed the dealWithNonDecayedTaus branch from b5b3171 to bbd0d87 Compare March 16, 2023 16:08

cmsbuild added tests-pending code-checks-pending and removed code-checks-approved labels Mar 16, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 16, 2023

cmsbuild added tests-started and removed tests-pending labels Mar 16, 2023

Dr15Jones changed the title ~~Make jets from taus which are not decayed in TauGenJetProducer~~ Make jets in TauGenJetProducer from taus which are not decayed Mar 16, 2023

perrotta reviewed Mar 16, 2023

View reviewed changes

cmsbuild added tests-approved and removed tests-started labels Mar 16, 2023

cmsbuild added orp-approved and removed orp-pending labels Mar 17, 2023

cmsbuild merged commit 6ab6e66 into cms-sw:master Mar 17, 2023

Dr15Jones deleted the dealWithNonDecayedTaus branch March 17, 2023 14:20

mbluj mentioned this pull request Mar 17, 2023

Add tau decay-mode to genTauJets as status #41092

Merged

cmsbuild mentioned this pull request Mar 17, 2023

Rename jemalloc libraries with debug and profiling options enabled. cms-sw/cmsdist#8389

Merged

makortel mentioned this pull request Mar 20, 2023

Maintenance tasks of week 11 cms-sw/framework-team#511

Closed

4 tasks

mbluj mentioned this pull request Mar 20, 2023

Build genTauJets for undecayed taus and add tau decay-mode to genTauJets [13_0_X] #41104

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make jets in TauGenJetProducer from taus which are not decayed #41064

Make jets in TauGenJetProducer from taus which are not decayed #41064

Dr15Jones commented Mar 15, 2023 •

edited

Loading

Dr15Jones commented Mar 15, 2023

Dr15Jones commented Mar 15, 2023

cmsbuild commented Mar 15, 2023

cmsbuild commented Mar 15, 2023

cmsbuild commented Mar 15, 2023

perrotta commented Mar 16, 2023

alebihan commented Mar 16, 2023

perrotta commented Mar 16, 2023

alebihan commented Mar 16, 2023

mbluj commented Mar 16, 2023

Dr15Jones commented Mar 16, 2023

perrotta commented Mar 16, 2023

mbluj commented Mar 16, 2023

alebihan commented Mar 16, 2023

mbluj commented Mar 16, 2023

cmsbuild commented Mar 16, 2023

cmsbuild commented Mar 16, 2023

Dr15Jones commented Mar 16, 2023

perrotta Mar 16, 2023

Dr15Jones Mar 16, 2023

cmsbuild commented Mar 16, 2023

perrotta commented Mar 17, 2023

alebihan commented Mar 17, 2023

mbluj commented Mar 17, 2023

perrotta commented Mar 17, 2023

perrotta commented Mar 17, 2023

perrotta commented Mar 19, 2023

mbluj commented Mar 20, 2023

Make jets in TauGenJetProducer from taus which are not decayed #41064

Make jets in TauGenJetProducer from taus which are not decayed #41064

Conversation

Dr15Jones commented Mar 15, 2023 • edited Loading

PR description:

PR validation:

Dr15Jones commented Mar 15, 2023

Dr15Jones commented Mar 15, 2023

cmsbuild commented Mar 15, 2023

cmsbuild commented Mar 15, 2023

cmsbuild commented Mar 15, 2023

Comparison Summary

perrotta commented Mar 16, 2023

alebihan commented Mar 16, 2023

perrotta commented Mar 16, 2023

alebihan commented Mar 16, 2023

mbluj commented Mar 16, 2023

Dr15Jones commented Mar 16, 2023

perrotta commented Mar 16, 2023

mbluj commented Mar 16, 2023

alebihan commented Mar 16, 2023

mbluj commented Mar 16, 2023

cmsbuild commented Mar 16, 2023

cmsbuild commented Mar 16, 2023

Dr15Jones commented Mar 16, 2023

perrotta Mar 16, 2023

Choose a reason for hiding this comment

Dr15Jones Mar 16, 2023

Choose a reason for hiding this comment

cmsbuild commented Mar 16, 2023

Comparison Summary

perrotta commented Mar 17, 2023

alebihan commented Mar 17, 2023

mbluj commented Mar 17, 2023

perrotta commented Mar 17, 2023

perrotta commented Mar 17, 2023

perrotta commented Mar 19, 2023

mbluj commented Mar 20, 2023

Dr15Jones commented Mar 15, 2023 •

edited

Loading