Fix generator interface to Geant4

[civanch]

PR description:

New problem is discussed after the fix #39427 was introduced. Now for displaced jets.
In this PR the previous condition for select predefined daughters decays is restored and a new one is added , which is less strict then one in #39427.

PR validation:

incomplete

if will be approved should be backported to MC production releases.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40820/34272

• This PR adds an extra 16KB to repository

[cmsbuild]

A new Pull Request was created by @civanch (Vladimir Ivantchenko) for master.

It involves the following packages:

• SimG4Core/Generators (simulation)

@cmsbuild, @civanch, @mdhildreth can you please review it and eventually sign? Thanks.
@makortel, @bsunanda, @rovere, @fabiocos, @slomeo 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]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40820/34273

• This PR adds an extra 16KB to repository

[civanch]

please test

[cmsbuild]

Pull request #40820 was updated. @civanch, @mdhildreth can you please check and sign again.

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-93c62e/30747/summary.html
COMMIT: eb9c804
CMSSW: CMSSW_13_1_X_2023-02-19-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40820/30747/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• You potentially added 27 lines to the logs
• Reco comparison results: 11 differences found in the comparisons
• DQMHistoTests: Total files compared: 49
• DQMHistoTests: Total histograms compared: 3529029
• DQMHistoTests: Total failures: 6
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3529001
• 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

[tvami]

@cmsbuild , please test workflow 11684.0

[mmusich]

I tested runTheMatrix.py --what upgrade -l 11684.0 -t 4 -j 8 --command='-n 50' with (red) and without this PR (blue) and changes seem to go in the right direction:

• less flat displaced fermion track efficiency
  
• less TrackingParticles overall
  

[tvami]

hold

• there is a different (more inclusive) proposal in Fix double counting in HepMC to G4 handoff #39427 (comment)
• If I see it correctly the displaced SUSY sample actually failed in this test

11684.0_DisplacedSUSY_stopToB_M_800_500mm_13+2021 Step0-DAS_ERROR Step1-NOTRUN Step2-NOTRUN Step3-NOTRUN Step4-NOTRUN  - time date Mon Feb 20 17:12:08 2023-date Mon Feb 20 17:12:06 2023; exit: 1 0 0 0 0

https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-93c62e/30756/dasqueries/failed_workflows.log

[cmsbuild]

Pull request has been put on hold by @tvami
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

[mmusich]

If I see it correctly the displaced SUSY sample actually failed in this test

it didn't fail. The DAS query failed.... I am not sure what we are actually probing there.
The worfklow per se did run to completion (successfully), cf: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-93c62e/30756/runTheMatrix-results/11684.0_DisplacedSUSY_stopToB_M_800_500mm_13+2021/

[mmusich]

there is a different (more inclusive) proposal in #39427 (comment)

what's the timeline for that proposal? I remind that the displaced SUSY physics is currently broken for 2022 / 2023 MC.

[mmusich]

if there is no consensus on the solution, perhaps it should be re-discussed at a SIM meeting? @civanch what do you think?

[tvami]

unhold

• maybe this Fix generator interface to Geant4 #40820 (comment) was too aggressive on me :) in the end I'm not the person who should make this call

what's the timeline for that proposal? I remind that the displaced SUSY physics is currently broken for 2022 / 2023 MC.

it would be nice if somebody from the displaced SUSY team would take initiative on the dedicated study comparing this PR and the solution proposed by Larry in the other thread.

[cmsbuild]

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 now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

[civanch]

SIM meeting is 10 days from now and usually there is no experts available. More appropriate would be Exotica meeting for discussion. I fully agree to start wide discussion on the interface but if SUSY physics is currently broken may be correct to merge this PR, make needed backport and when better solution will be available make a new one.

We also need to move discussion from 39427 to the new git issue.

[lawrenceleejr]

I really don’t know if my proposal in the other PR is a solution to this problem. I also don’t know if this PR is the solution to this problem. The plots are suggestive which is good. But looking at the change, this reintroduces the bug in the SM samples from the other PR. So that’s definitely not the way to go.

It also reintroduced stau-specific code here which I think is a dangerous path to go down again.

I do think this needs to be figured out and fixed in a more sustainable way, but as I said in the other thread, I don’t have the person power at this time to help make that happen. So if we can find a halfway solution like this (but not quite this one…) then I’d be fine with that. But as @tvami says — it’s not really our call… but this reverting of the other PR will affect SM samples.

[mmusich]

More appropriate would be Exotica meeting for discussion. I fully agree to start wide discussion on the interface but if SUSY physics is currently broken may be correct to merge this PR, make needed backport and when better solution will be available make a new one.

@CeliaFernandez can you perhaps circulate this proposal to the relevant e-groups?

[CeliaFernandez]

More appropriate would be Exotica meeting for discussion. I fully agree to start wide discussion on the interface but if SUSY physics is currently broken may be correct to merge this PR, make needed backport and when better solution will be available make a new one.

@CeliaFernandez can you perhaps circulate this proposal to the relevant e-groups?

Sure!

[SohamBhattacharya]

Hi all, does this fix affect long-lived staus (LLStaus), or is only relevant for processed with "displaced" signatures like R-hadrons, HNLs, etc?
We are soon going to submit a Run-2 UL sample request for LLStaus and if this affects the LLStau signature, we would like to request a backport to CMSSW_10_6_31.

Tagging @shedprog , @sarafiorendi .

[perrotta]

@civanch @CeliaFernandez the target for 13_0_0 is in 4 days from now, and this is only the master PR that will have to be possibly backported after having been merged: do you have an ETA for the conclusions of your checks, or at least for a (motivated) decision about whether to merge this PR?

[mmusich]

the target for 13_0_0 is in 4 days from now, and this is only the master PR that will have to be possibly backported after having been merged: do you have an ETA for the conclusions of your checks, or at least for a (motivated) decision about whether to merge this PR?

AFAIK, the issue is going to be discussed on Feb 28th at the EXO general meeting. Perhaps some consensus or roadmap will be reached there.
I just wanted to mention - nevertheless - that people doing analysis are starting to notice the problem (see e.g.: cmsTalk thread)

[CeliaFernandez]

@civanch @CeliaFernandez the target for 13_0_0 is in 4 days from now, and this is only the master PR that will have to be possibly backported after having been merged: do you have an ETA for the conclusions of your checks, or at least for a (motivated) decision about whether to merge this PR?

Hi @perrotta , I just wanted to clarify that I'm not currently doing any further checks on this. Unfortunately I'm not an expert on this part of the code and I don't feel qualified at this point to go through it. I simply noticed that something weird was going on with this sample when doing the validation. There will be a meeting next Tuesday to discuss this with the relevant people and I hope that can help to clarify to way to go.

[zdemirag]

Hello all, we want to highlight to all in this thread that Celia will summarize the issue in the EXO General where we can discuss it all together: https://indico.cern.ch/event/1242612/#2-geant4-issue-in-handling-sim

Thanks!
Livia, Zeynep

[perrotta]

+1

• As stated at the EXO meeting on Feb 28:
  • It reintroduces the bug that was removed in the initial PR.
  • But DisplacedSUSY is currently broken and would need to be fixed.
• Therefore, it was decided at the ORP of the same day that this can be merged in 13_1_X as well as in 13_0_X. For 12_4_X and 12_6_X,, where the productions have already started, a proc modifier would be needed instead.

[DickyChant]

Hi all, does this fix affect long-lived staus (LLStaus), or is only relevant for processed with "displaced" signatures like R-hadrons, HNLs, etc? We are soon going to submit a Run-2 UL sample request for LLStaus and if this affects the LLStau signature, we would like to request a backport to CMSSW_10_6_31.

Tagging @shedprog , @sarafiorendi .

We want to ask if this PR will be back-ported to 12_4_X and 10_6_X? From EXO, currently there is no related request running, but we may need this fix for upcoming signal requests.

[SohamBhattacharya]

Hi all, does this fix affect long-lived staus (LLStaus), or is only relevant for processed with "displaced" signatures like R-hadrons, HNLs, etc? We are soon going to submit a Run-2 UL sample request for LLStaus and if this affects the LLStau signature, we would like to request a backport to CMSSW_10_6_31.

Tagging @shedprog , @sarafiorendi .

Following up on this:

I simulated some long-lived stau (m=250GeV, lifetime ctau0=100mm) events using 10_6_31_patch1 (for UL18).
Looking at the SimTracks table (with pt > 10 GeV) in the GEN-SIM file (run 1, lumi 1, event 34), I do not see any obvious duplication of prompt particles like that reported in [0].

To check if this PR affects us, I modified this [1] in 10_6_31_patch1 to the following, to match the change [2] made in this PR:

    bool checkStatus = fFinalState ? (((*vpdec)->status() == 23 && std::abs(vp->pdg_id()) == 1000015) || ((*vpdec)->status() > 50 && (*vpdec)->status() < 100))
      : ((*vpdec)->status() == 23 && std::abs(vp->pdg_id()) == 1000015);

Note that we turn on fFinalState by passing --procModifiers run2_final_state_rad to cmsDriver.
Looking at the same event as above (we used the same seed in the simulation, so that a 1-to-1 comparison can be made), I see it's identical:

So I'm tempted to say that the long-lived stau simulation in default 10_6_31_patch1 is okay, and this PR would not make any difference to us if it were backported.
However, I'm not an expert on the simulation details discussed here, so please let me know if I've missed something.

In case anyone wants to take a look, our GEN-SIM, RECO, and MINIAOD files are in [3] for default 10_6_31_patch1, and in [4] for 10_6_31_patch1 with the aforementioned change.

[0] #39427 (comment)
[1] https://github.com/cms-sw/cmssw/blob/CMSSW_10_6_31_patch1/SimG4Core/Generators/src/Generator.cc#L455-L456
[2] https://github.com/cms-sw/cmssw/pull/40820/files#diff-6597e4af9316e534d1f2b08ab6b907f11ec1a4a52cbfd2460dd234d2614712b0R478-R479
[3] /eos/user/s/sobhatta/LongLivedStaus/test/test_LLSTauProduction_GEANT4fix/default/
[4] /eos/user/s/sobhatta/LongLivedStaus/test/test_LLSTauProduction_GEANT4fix/new_with-PR40820/