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

Backport of "Fix for FastSim decays of exotic-descendent SM particles decaying outside pipe" to 9_4_X #36354

Merged
merged 6 commits into from
Jan 18, 2022

Conversation

cericeci
Copy link

@cericeci cericeci commented Dec 3, 2021

PR description:

Backport of #36122 and #36324 for preUL 2017 FastSim production. Full functionality of both, without the code formatting. Please check #36352 (backport to UL branch) first

This fixes a bug which would invalidate simulation for long-lived and a fraction of prompt signals (and luckily was introduced very recently, so most of the previous production is safe). Our understanding is that this case doesn't go against the no-change policy.

Original PR description: Jaebak found that some b- and d-mesons produced in the decays of exotic particles, which cross the beam pipe radius, have their decay performed twice, with sim hits created for each copy. This causes problems for some high-pT b-jets, as well as the MET in events that contain such jets. A few lines are added to ParticleManager.cc which results in FastSim deciding to not decay such particles further, and rather take only the original generator-determined decay trees. FastSim will still propagate these particles and create secondaries/material interactions, but it won't decay them.

If this PR is a backport please specify the original PR and why you need to backport that PR:

#36122 and #36324, needed for preUL 2017 MC production (most notably pMSSM scan)

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 3, 2021

A new Pull Request was created by @cericeci (Carlos Erice) for CMSSW_9_4_X.

It involves the following packages:

  • FastSimulation/SimplifiedGeometryPropagator (fastsim)

@ssekmen, @lveldere, @civanch, @mdhildreth, @cmsbuild, @sbein can you please review it and eventually sign? Thanks.
@matt-komm this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@perrotta
Copy link
Contributor

perrotta commented Dec 3, 2021

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 3, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6f8905/20970/summary.html
COMMIT: 6d344a3
CMSSW: CMSSW_9_4_X_2021-11-28-0000/slc6_amd64_gcc630
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36354/20970/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 27
  • DQMHistoTests: Total histograms compared: 2721493
  • DQMHistoTests: Total failures: 113
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2721218
  • DQMHistoTests: Total skipped: 162
  • DQMHistoTests: Total Missing objects: 0
  • TriggerResults: no differences found

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 6, 2021

This pull request is fully signed and it will be integrated in one of the next CMSSW_9_4_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_2_X is complete. This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@perrotta
Copy link
Contributor

-1
This PR is "fully signed", but it misses the necessary customization with the modifiers if it wants to be backported in a closed production release. In order not to merge it "by mistake" let have it orp-rejected by now. The flag will be overwritten if and when a modification will be implemented in the PR

@cmsbuild
Copy link
Contributor

Pull request #36354 was updated. @perrotta, @ssekmen, @lveldere, @civanch, @mdhildreth, @cmsbuild, @sbein, @qliphy, @fabiocos, @davidlange6 can you please check and sign again.

@cericeci
Copy link
Author

Same as #36353, but checked on CMSSW_9_4_X_2022-01-09-0000 instead

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6f8905/21609/summary.html
COMMIT: a6793f1
CMSSW: CMSSW_9_4_X_2022-01-09-0000/slc6_amd64_gcc630
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36354/21609/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 27
  • DQMHistoTests: Total histograms compared: 2721493
  • DQMHistoTests: Total failures: 112
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2721219
  • DQMHistoTests: Total skipped: 162
  • DQMHistoTests: Total Missing objects: 0
  • TriggerResults: no differences found

@qliphy
Copy link
Contributor

qliphy commented Jan 13, 2022

ping @cms-sw/fastsim-l2

@sbein
Copy link
Contributor

sbein commented Jan 17, 2022

+1

@perrotta
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_9_4_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_3_X is complete. This pull request will be automatically merged.

@cericeci
Copy link
Author

Hi @perrotta, thanks for merging this one. What is the policy/estimated time for this to make it into a new release? As I said at the start this would be needed for a few lingering preUL samples (namely pMSSM scan, that involves already finished analysis that used preUL). Should we bring this discussion to PPD instead?

@perrotta
Copy link
Contributor

Hi @perrotta, thanks for merging this one. What is the policy/estimated time for this to make it into a new release? As I said at the start this would be needed for a few lingering preUL samples (namely pMSSM scan, that involves already finished analysis that used preUL). Should we bring this discussion to PPD instead?

@cericeci I've started building CMSSW_9_4_21_patch1 and CMSSW_10_2_28 with these fixes in
They will become available either by this evening or tomorrow

@cericeci
Copy link
Author

Many thanks @perrotta !

@perrotta
Copy link
Contributor

Even faster that I could imagine:
https://github.com/cms-sw/cmssw/releases/CMSSW_9_4_21_patch1

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.

5 participants