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

adding option to enable FastSim decayer functionality #38813

Merged
merged 2 commits into from Jul 24, 2022

Conversation

sbein
Copy link
Contributor

@sbein sbein commented Jul 21, 2022

This PR creates an option to enable the FastSim decayer functionality, which was previously enabled without an option. The decayer has been found to have been responsible for considerable mismodeling in the missing ET, a known issue with a heavy-handed correction procedure developed by and adopted by the SUS PAG. The impact of the FastSim decayer on physics was recently studied and presented in the simulation meeting [1].

[1] https://indico.cern.ch/event/1182398/contributions/4967736/attachments/2480801/4258724/Sim15July2022_Decayer.pdf

Adopting the GEN decays has many advantages, including the guaranteed synchronization of decay modes btw the generator and simulation steps, the universal ability to allow for the decays of exotic particles without analyzing the family tree of every particle, and avoiding any unforeseen consequences of switching generators, e.g., from Pythia8 to Herwig. Most importantly, defaulting to the GEN decays was seen to improve the modeling of MET and other observables in FastSim significantly, while any bad impacts are for the most part minor [2][3]

The SUSY-derived FastSim MET correction may no longer be necessary, but it is up to PAGs to assess any treatment/uncertainties they think are appropriate for their analyses. It would also be advisable to backport this default behavior to the Run 2 UL release, CMSSW_10_6_X for any future campaigns.

PR validation:

[ALT_0] is the new FastSim:
[2] https://www.desy.de/~beinsam/FastSim/Nano/6July2022/TTbar_12_2_XDev/index.html
[3] https://www.desy.de/~beinsam/FastSim/Nano/6July2022/T1tttt_12_2_XDev/index.html

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38813/31183

  • This PR adds an extra 24KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38813/31186

  • This PR adds an extra 24KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @sbein (Sam Bein) for master.

It involves the following packages:

  • FastSimulation/SimplifiedGeometryPropagator (fastsim)

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

cms-bot commands are listed here

@sbein sbein changed the title adding option to enable FastSim decayer functionality, where before i… adding option to enable FastSim decayer functionality Jul 21, 2022
@sbein
Copy link
Contributor Author

sbein commented Jul 21, 2022

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9fbcba/26371/summary.html
COMMIT: 4436737
CMSSW: CMSSW_12_5_X_2022-07-21-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/38813/26371/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-9fbcba/13234.0_TTbar_14TeV+2021FS+TTbar_14TeV_TuneCP5_FastSimRun3+HARVESTFastRun3
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-9fbcba/13434.0_TTbar_14TeV+2021FSPU+TTbar_14TeV_TuneCP5_FastSimRun3PU+HARVESTFastRun3PU

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 5254 differences found in the comparisons
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3706484
  • DQMHistoTests: Total failures: 52999
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 3653462
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.004 KiB( 50 files compared)
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 210 log files, 47 edm output root files, 51 DQM output files
  • TriggerResults: found differences in 3 / 50 workflows

@sbein
Copy link
Contributor Author

sbein commented Jul 21, 2022

+1

@cmsbuild
Copy link
Contributor

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, @qliphy, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

@perrotta
Copy link
Contributor

While I interpret the fastsim signature here as that all differences in the fastsim workflows are approved as a consequence of this change, I wonder what do these two workflows, listed under "Workflows with reco comparison differences", correspond to
all_OldVSNew_TTbarFS13in2018RECOwf2018p1
all_OldVSNew_TTbarFS13in2018MINIAODwf2018p1

I imagine that the final field in the name, that should correspond to the workflow number (here 2018.1 for both), got somehow screwed up: @slava77 do you have any idea why?

@slava77
Copy link
Contributor

slava77 commented Jul 23, 2022

correspond to all_OldVSNew_TTbarFS13in2018RECOwf2018p1 all_OldVSNew_TTbarFS13in2018MINIAODwf2018p1

I imagine that the final field in the name, that should correspond to the workflow number (here 2018.1 for both), got somehow screwed up: @slava77 do you have any idea why?

I'm not sure what is the question.
AOD/RECO and miniAOD files are processed separately and go to separate foldets. In this workflow, unlike in many regular pp workflows the RECO and PAT steps are in separate steps. So, the pattern of the output folder names is different.

@perrotta
Copy link
Contributor

correspond to all_OldVSNew_TTbarFS13in2018RECOwf2018p1 all_OldVSNew_TTbarFS13in2018MINIAODwf2018p1
I imagine that the final field in the name, that should correspond to the workflow number (here 2018.1 for both), got somehow screwed up: @slava77 do you have any idea why?

I'm not sure what is the question. AOD/RECO and miniAOD files are processed separately and go to separate foldets. In this workflow, unlike in many regular pp workflows the RECO and PAT steps are in separate steps. So, the pattern of the output folder names is different.

Sorry, Slava: I did not realize that "2018.1" was exactly the number of the workflow: I imagined that the year instead was slipped somehow int the report name. Sorry for the noise, then

@perrotta
Copy link
Contributor

+1

  • Differences wrt the baseline are quite relevant, as expected.
  • Let profit of next 12_5_X pre-release to have it more extensively validated

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.

None yet

4 participants