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

HCAL: configurable option to use actual Run3 HF ShowerLibrary for Run2 MC #40357

Merged
merged 2 commits into from Dec 19, 2022

Conversation

abdoulline
Copy link

@abdoulline abdoulline commented Dec 17, 2022

PR description:

Follow up on the issue #40218
After discussion in Simulation meeting on Friday, Dec.16
https://indico.cern.ch/event/1230771/
-> https://indico.cern.ch/event/1230771/contributions/5185071/attachments/2568386/4428907/Run2_HFShowerLibrary_SIM_Dec12_2022.pdf

The decision has been taken: to have a configurable option to use actual Run3 HFSHowerLibrary file for Run2 simulation.

@Dr15Jones and @makortel suggested to use modifier for enabling the option:

(1 ) with cmsDriver this option can be enabled with
--procModifiers applyHFLibraryFix

(2) In the job config may look (specifically for Run2_2018 era in the example below) like this:

from Configuration.Eras.Era_Run2_2018_cff import Run2_2018
from Configuration.ProcessModifiers.applyHFLibraryFix_cff import applyHFLibraryFix
process = cms.Process('SIM',Run2_2018,applyHFLibraryFix)

PR validation:

(1) without option activation (default) : runTheMatrix.py -l limited
(2) with activation : using wf 10824.0_TTbar_13+2018

If this PR is a backport

No

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40357/33451

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @abdoulline (Salavat Abdullin) for master.

It involves the following packages:

  • Configuration/ProcessModifiers (operations)
  • Geometry/HcalSimData (geometry)
  • SimCalorimetry/HcalSimProducers (simulation)

@perrotta, @rappoccio, @Dr15Jones, @makortel, @ianna, @mdhildreth, @cmsbuild, @bsunanda, @civanch, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@missirol, @makortel, @rovere, @Martin-Grunewald, @bsunanda, @mariadalfonso, @sameasy, @fabiocos 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

@civanch
Copy link
Contributor

civanch commented Dec 18, 2022

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-16a119/29677/summary.html
COMMIT: 4120490
CMSSW: CMSSW_13_0_X_2022-12-17-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40357/29677/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: 5 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3555748
  • DQMHistoTests: Total failures: 157
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3555569
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 211 log files, 162 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@civanch
Copy link
Contributor

civanch commented Dec 19, 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 master IBs (tests are also fine). This pull request will be automatically merged.

@cmsbuild cmsbuild merged commit 51e2735 into cms-sw:master Dec 19, 2022
#
#--- Alternative: to use Run3 library with applyHFLibraryFix modifier
(applyHFLibraryFix & run2_common).toModify( HFLibraryFileBlock, FileName = 'SimG4CMS/Calo/data/HFShowerLibrary_run3_v6.root', FileVersion = 2 )
(applyHFLibraryFix & run2_common).toModify( HFShowerBlock, EqualizeTimeShift = True )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to stir the pot after merging, but I just want to point out that in this file the impact of applyHFLibraryFix and run3_HFSL modifiers is the same (except applyHFLibraryFix gets applied only if run2_common is enabled as well, whereas run3_HFSL acts independently).

hf2 = dict( samplingFactor = 0.37,
timePhase = 8.0
)
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the other hand, in here these customizations made with applyHFLibraryFix are also done as part of customizations made with run3_common modifier further below.

While this arrangement works technically, I'm concerned at some point this "duplication" would cause confusion. An alternative would be to make the necessary customizations only with run3_HFSL modifier. In this way the fix could be enabled with cmsDriver.py's --eras parameter along--eras Run2_2018,run3_HFSL. A downside is that the purpose of using run3_HFSL in this way would be less clear without additional documentation.

@abdoulline
Copy link
Author

abdoulline commented Dec 19, 2022

Hi @makortel thanks for the comments.
As run3_HFSL is a part of Run3 era and run2_common is inherited by Run3 era, I did see some duplication ☹
But as you've said - technically it works and I didn't find more elegant solution...
At least the parameters are explicitly set (even if duplicated) in each case, so reading/interpreting each case should be easy, I guess (?).

@abdoulline abdoulline deleted the applyHFLibraryFix_Run3_to_Run2 branch April 18, 2023 05:36
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

5 participants