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

[12_4_X] Update Run3 MC GTs + swap update Vertex smearing + update offline Run3 GTs #39019

Merged

Conversation

francescobrivio
Copy link
Contributor

@francescobrivio francescobrivio commented Aug 10, 2022

PR description:

Backport of:

This PR is meant to generally update the GTs in 12_4_X to make them even with respect to 12_5_X and 12_6_X.
The PR includes the swap of Run3RoundOptics25ns13TeVLowSigmaZ with Realistic25ns13p6TeVEarly2022Collision smearing so that the tests also use the correct new vertex smearing (GS file added in #39548)

New tags included:

  • Tags for auto:phase1_2022_realistic:
    • BeamSpotObjects_Realistic25ns_13p6TeVCollisions_Early2022_v3_mc
    • BeamSpotOnlineObjects_Realistic25ns_13p6TeVCollisions_Early2022_v2_mc
    • SiStripBadComponents_realisticMC_for2022_v2_mc
    • SiPixelQuality_phase1_2022_v2_mc
    • SiPixelQuality_forDigitizer_phase1_2022_v2_mc with label forDigitizer`
    • SiPixelQuality_phase1_2022_forRawToDigi_v0 with label forRawToDigi`
  • Tags for auto:phase1_2022_cosmics:
    • BeamSpotObjects_Realistic25ns_13p6TeVCollisions_Early2022_v3_mc
    • BeamSpotOnlineObjects_Realistic25ns_13p6TeVCollisions_Early2022_v2_mc
    • SiStripBadComponents_realisticMC_for2022_v2_mc
    • SiPixelQuality_phase1_2022_v2_mc
    • SiPixelQuality_forDigitizer_phase1_2022_v2_mc with label forDigitizer
  • Tags for auto:phase1_2022_realistic_hi:
    • SiStripBadComponents_realisticMC_for2022_v2_mc
    • SiPixelQuality_phase1_2022_v2_mc
    • SiPixelQuality_forDigitizer_phase1_2022_v2_mc with label forDigitizer
  • Tags for auto:phase1_2023_realistic:
    • SiStripBadComponents_realisticMC_for2022_v2_mc
    • SiPixelQuality_phase1_2023_v2_mc
    • SiPixelQuality_forDigitizer_phase1_2023_v2_mc with label forDigitizer
  • Tags for auto:phase1_2024_realistic:
    • SiStripBadComponents_realisticMC_for2022_v2_mc
    • SiPixelQuality_phase1_2024_v2_mc
    • SiPixelQuality_forDigitizer_phase1_2024_v2_mc with label forDigitizer
  • Tags for auto:run3_data
    • too many changes to be listed here, see diff below
  • Tags for auto:run3_data_relval:
    • identical to auto:run3_data with fixed L1 menu

GT Diffs:

PR validation:

Code compiles - ran the tests in the master PR

Backport:

Backport of #38760, #39645 and #39823

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @francescobrivio for CMSSW_12_4_X.

It involves the following packages:

  • Configuration/AlCa (alca)
  • Configuration/PyReleaseValidation (pdmv, upgrade)
  • EventFilter/CSCRawToDigi (reconstruction)
  • Geometry/CMSCommonData (geometry, upgrade)
  • Geometry/HcalCommonData (geometry)
  • Geometry/TrackerCommonData (geometry)
  • Geometry/VeryForwardData (geometry)
  • SimG4CMS/Calo (simulation)
  • SimG4CMS/ShowerLibraryProducer (simulation)
  • SimG4Core/Configuration (simulation)

@malbouis, @civanch, @yuanchao, @jordan-martins, @bsunanda, @makortel, @bbilin, @saumyaphor4252, @ianna, @mdhildreth, @cmsbuild, @AdrianoDee, @srimanob, @Dr15Jones, @clacaputo, @kskovpen, @jpata, @tvami, @ChrisMisan, @francescobrivio can you please review it and eventually sign? Thanks.
@VourMa, @felicepantaleo, @kpedro88, @ghugo83, @Martin-Grunewald, @bsunanda, @grzanka, @trtomei, @slomeo, @venturia, @vargasa, @makortel, @JanFSchulte, @missirol, @simonepigazzini, @thomreis, @beaucero, @barvic, @fabferro, @rovere, @VinInn, @ptcox, @tocheng, @mmusich, @mtosi, @fabiocos 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

@mmusich
Copy link
Contributor

mmusich commented Aug 10, 2022

Who'll be the primary user of this?

@tvami
Copy link
Contributor

tvami commented Aug 10, 2022

Who'll be the primary user of this?

HLT studies

@Martin-Grunewald
Copy link
Contributor

Sorry, but I suppose this is for the 12_4 MC production, no?

@tvami
Copy link
Contributor

tvami commented Aug 10, 2022

Sorry, but I suppose this is for the 12_4 MC production, no?

Yes, this is used in the 12_4_X MC production. On the top of this change, we'll prove you @Martin-Grunewald et al special GTs that are including the JEC + PF for your autoCondHLT

@tvami
Copy link
Contributor

tvami commented Aug 10, 2022

@cmsbuild , please test

@tvami
Copy link
Contributor

tvami commented Aug 10, 2022

@cmsbuild , please abort

  • I forgot that we need input from PdmV

@mmusich
Copy link
Contributor

mmusich commented Aug 10, 2022

Sorry, but I suppose this is for the 12_4 MC production, no?

Why does it need to be in release? Production does not care

@mmusich
Copy link
Contributor

mmusich commented Aug 10, 2022

hold

  • until someone convinces me otherwise

@cmsbuild
Copy link
Contributor

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

@cmsbuild cmsbuild added the hold label Aug 10, 2022
@tvami
Copy link
Contributor

tvami commented Aug 10, 2022

Marco, this was agreed by PPD L1s, and ORP yesterday. Sal, Jordan, Kaori should comment...

@mmusich
Copy link
Contributor

mmusich commented Aug 10, 2022

Please let's have a written justification. IIUC there are no minutes of the ORP meeting

@fwyzard
Copy link
Contributor

fwyzard commented Aug 10, 2022

As far as I kow the ORP meeting yesterday had been cancelled:

From: Qiang Li qliphy@gmail.com
To: cms-release-dataops@cern.ch
Subject: ORP[2022/08/09] cancelled

Dear all,

As the title says, we cancel this week's ORP meeting as many people are on
vacations.

If you have any urgent requests, please fell free write to let us know.
Otherwise we can also discuss relevant stuff at the joint Ops meeting this
week.

Kindly reminds you the next milestone
CMSSW_12_5_0_pre5: 2022/08/23 (last open pre-release)

Best,
Andrea, Qiang

@rappoccio
Copy link
Contributor

@mmusich we didn't have an ORP yesterday but we discussed privately with @qliphy and this was agreed to add for posterity and ease of use.

@mmusich
Copy link
Contributor

mmusich commented Aug 10, 2022

@rappoccio,

ease of use

use for what reason? Why are we changing the physics output of a closed release?

@mandrenguyen
Copy link
Contributor

@bsunanda @mandrenguyen @civanch now that #39548 is merged, to be consistent this should be merged as well , would you please review and sign? thanks!

For reco, @clacaputo was looking at this one.

@clacaputo
Copy link
Contributor

+reconstruction

@bsunanda
Copy link
Contributor

+geometry

@tvami
Copy link
Contributor

tvami commented Nov 30, 2022

@bsunanda maybe you could also sign for simulation?

@bsunanda
Copy link
Contributor

bsunanda commented Dec 2, 2022

+1

@bsunanda
Copy link
Contributor

bsunanda commented Dec 2, 2022

@civanch @tvami I believe Vladimir or Mike (Hildredth) can sign for simulation

@civanch
Copy link
Contributor

civanch commented Dec 2, 2022

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 2, 2022

This pull request is fully signed and it will be integrated in one of the next CMSSW_12_4_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_13_0_X is complete. 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)

@tvami
Copy link
Contributor

tvami commented Dec 2, 2022

hi @cms-sw/orp-l2
I had this comment earlier

now that #39548 is merged, to be consistent this should be merged as well

in the last few IBs we had inconsistencies so it would be nice to merge this as soon as possible, thanks!

@perrotta
Copy link
Contributor

perrotta commented Dec 2, 2022

please test
(9 days old tests... just check that there is no inconsistency with #39548 or any other PR merged since then: the plan is to merge as soon as the tests succeed, quite likely sometime tomorrow morning)

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 3, 2022

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fd62c0/29433/summary.html
COMMIT: fcb9b1b
CMSSW: CMSSW_12_4_X_2022-11-29-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/39019/29433/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
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 22728 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 3709306
  • DQMHistoTests: Total failures: 50066
  • DQMHistoTests: Total nulls: 3
  • DQMHistoTests: Total successes: 3659215
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 5.189 KiB( 49 files compared)
  • DQMHistoSizes: changed ( 11834.0 ): 5.189 KiB SiStrip/MechanicalView
  • Checked 208 log files, 161 edm output root files, 50 DQM output files
  • TriggerResults: found differences in 7 / 49 workflows

@perrotta
Copy link
Contributor

perrotta commented Dec 3, 2022

+1

@cmsbuild cmsbuild merged commit c7f7797 into cms-sw:CMSSW_12_4_X Dec 3, 2022
@@ -34,7 +34,7 @@
# step1 gensim: for 2018 HI prod
step1Up2018HiProdDefaults = merge ([{'--eventcontent':'RAWSIM'},step1Up2018HiDefaults])
# step1 gensim: for 2021 HI prod
step1Up2021HiProdDefaults = merge ([{'--conditions':'auto:phase1_2022_realistic_hi','--era':'Run3_pp_on_PbPb','--beamspot':'Run3RoundOptics25ns13TeVLowSigmaZ','--eventcontent':'RAWSIM','--geometry':'DB:Extended'},step1Up2018HiDefaults])
step1Up2021HiProdDefaults = merge ([{'--conditions':'auto:phase1_2022_realistic_hi','--era':'Run3_pp_on_PbPb','--beamspot':'Realistic25ns13p6TeVEarly2022Collision','--eventcontent':'RAWSIM','--geometry':'DB:Extended'},step1Up2018HiDefaults])
Copy link
Contributor

Choose a reason for hiding this comment

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

@cms-sw/alca-l2

I'm having trouble understanding why this change is needed (and two other ones below).

After this PR, the MC GTs for 2022_hi, 2023, and 2024 [1] still use the 'old' beamspot tags, i.e. Run3RoundOptics25ns13TeVLowSigmaZ (iiuc, this is because the PU samples of the corresponding RelVals were produced with the 'old' beamspot). At the same time, this PR changes the beamspot parameters used in the GEN-SIM step of those workflows to the 'new' one, i.e. Realistic25ns13p6TeVEarly2022Collision.

Taking wf 12434.0 as an example (pp 2023, no PU), step-1 is now as in [2], and this plot from the PR tests looks odd.

Am I missing something?

[1]

[2]

cmsDriver.py TTbar_14TeV_TuneCP5_cfi  -s GEN,SIM -n 10 \
 --conditions auto:phase1_2023_realistic --beamspot Realistic25ns13p6TeVEarly2022Collision \
 --datatier GEN-SIM --eventcontent FEVTDEBUG --geometry DB:Extended --era Run3 --relval 9000,100

Copy link
Contributor

@mmusich mmusich Dec 5, 2022

Choose a reason for hiding this comment

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

This looks like a mistake.
In the master version of this PR: #38760 the >= 2022 scenarios got updated because the corresponding PR updating the PU libraries has changed the later Run-3 years as well (see #39041) but in this case this is not appropriate.
I suggest this bug-fix PR #40238 to resolve the issue.
Alternatively we'd need updates to GlobalTags as well as PU libraries.

@@ -2002,15 +2002,15 @@ def condition(self, fragment, stepList, key, hasHarvest):
'GT' : 'auto:phase1_2023_realistic',
'HLTmenu': '@relval2022',
'Era' : 'Run3',
'BeamSpot': 'Run3RoundOptics25ns13TeVLowSigmaZ',
'BeamSpot': 'Realistic25ns13p6TeVEarly2022Collision',
Copy link
Contributor

Choose a reason for hiding this comment

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

See first comment.

'ScenToRun' : ['GenSim','Digi','RecoNano','HARVESTNano','ALCA'],
},
'2024' : {
'Geom' : 'DB:Extended',
'GT' : 'auto:phase1_2024_realistic',
'HLTmenu': '@relval2022',
'Era' : 'Run3',
'BeamSpot': 'Run3RoundOptics25ns13TeVLowSigmaZ',
'BeamSpot': 'Realistic25ns13p6TeVEarly2022Collision',
Copy link
Contributor

Choose a reason for hiding this comment

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

See first comment.

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