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.

Sign up for GitHub

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

Jump to bottom

[L1T] Phase-2, remove old simulation and adapt DQM #38442

Merged
merged 73 commits into from
Aug 25, 2022
Merged

[L1T] Phase-2, remove old simulation and adapt DQM #38442

merged 73 commits into from
Aug 25, 2022

Conversation

cecilecaillol
Copy link
Contributor

@cecilecaillol cecilecaillol commented Jun 20, 2022 •
edited
Loading

PR description:

This PR switches to the new Phase-2 emulators and removes the old simulations. It also fixes the PF DQM problem and adds DQM for muons.
HLT also switched to new objects (@trtomei )
This PR includes a bug fix to the vertex producer to make the simulation match the FW/emulator as documented on S4 of these slides https://indico.cern.ch/event/1139008/#10-tracks-vertex

@cmsbuild cmsbuild added this to the CMSSW_12_5_X milestone Jun 20, 2022
@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38442/30640

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-38442/30641

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @cecilecaillol for master.

It involves the following packages:

  • DQMOffline/L1Trigger (dqm, l1)
  • DataFormats/L1TCorrelator (upgrade, l1)
  • DataFormats/L1TParticleFlow (upgrade, l1)
  • DataFormats/L1TrackTrigger (upgrade, l1)
  • DataFormats/L1Trigger (l1)
  • L1Trigger/Configuration (l1)
  • L1Trigger/L1CaloTrigger (upgrade, l1)
  • L1Trigger/L1TTrackMatch (upgrade, l1)
  • L1Trigger/Phase2L1GMT (l1)
  • L1Trigger/Phase2L1ParticleFlow (upgrade, l1)
  • L1Trigger/Phase2L1Taus (l1)
  • L1Trigger/VertexFinder (l1)

@rekovic, @epalencia, @emanueleusai, @ahmad3213, @cmsbuild, @AdrianoDee, @srimanob, @jfernan2, @pmandrik, @micsucmed, @cecilecaillol, @rvenditti can you please review it and eventually sign? Thanks.
@kreczko, @sviret, @eyigitba, @Martin-Grunewald, @missirol, @dinyar, @thomreis, @trtomei, @rovere, @beaucero, @rociovilar 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

@cecilecaillol
Copy link
Contributor Author

please test with cms-data/L1Trigger-Phase2L1ParticleFlow#4

@cmsbuild
Copy link
Contributor

-1

Failed Tests: UnitTests RelVals RelVals-INPUT AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6d4179/25643/summary.html
COMMIT: 64cbf05
CMSSW: CMSSW_12_5_X_2022-06-20-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/38442/25643/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6d4179/25643/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6d4179/25643/git-merge-result

Unit Tests

I found errors in the following unit tests:

---> test testRecoMETMETProducers had ERRORS

RelVals

  • 136.731136.731_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT_skimSinglePh_HIPM+HARVESTDR2_skimSinglePh/step2_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT_skimSinglePh_HIPM+HARVESTDR2_skimSinglePh.log
  • 5.15.1_TTbar+TTbarFS+HARVESTFS/step1_TTbar+TTbarFS+HARVESTFS.log
  • 135.4135.4_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS/step1_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS.log
Expand to see more relval errors ...

RelVals-INPUT

  • 4.224.22_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC/step2_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC.log
  • 4.64.6_MinimumBias2010A+MinimumBias2010A+RECOSKIMALCA+HARVESTDR1/step2_MinimumBias2010A+MinimumBias2010A+RECOSKIMALCA+HARVESTDR1.log
  • 6.06.0_SingleMuPt1+SingleMuPt1INPUT+DIGI+RECO+HARVEST/step2_SingleMuPt1+SingleMuPt1INPUT+DIGI+RECO+HARVEST.log
Expand to see more relval errors ...

AddOn Tests

  • fastsimcmsDriver.py TTbar_8TeV_TuneCUETP8M1_cfi --conditions auto:run1_mc --fast -n 100 --eventcontent AODSIM,DQM --relval 100000,1000 -s GEN,SIM,RECOBEFMIX,DIGI:pdigi_valid,L1,DIGI2RAW,L1Reco,RECO,VALIDATION --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --datatier GEN-SIM-DIGI-RECO,DQMIO --beamspot Realistic8TeVCollision : FAILED - time: date Mon Jun 20 18:51:47 2022-date Mon Jun 20 18:51:35 2022 s - exit: 256
  • fastsim1cmsDriver.py TTbar_13TeV_TuneCUETP8M1_cfi --conditions auto:run2_mc_l1stage1 --fast -n 100 --eventcontent AODSIM,DQM --relval 100000,1000 -s GEN,SIM,RECOBEFMIX,DIGI:pdigi_valid,L1,DIGI2RAW,L1Reco,RECO,VALIDATION --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --datatier GEN-SIM-DIGI-RECO,DQMIO --beamspot NominalCollision2015 --era Run2_25ns : FAILED - time: date Mon Jun 20 18:51:47 2022-date Mon Jun 20 18:51:40 2022 s - exit: 256
  • fastsim2cmsDriver.py TTbar_13TeV_TuneCUETP8M1_cfi --conditions auto:run2_mc --fast -n 100 --eventcontent AODSIM,DQM --relval 100000,1000 -s GEN,SIM,RECOBEFMIX,DIGI:pdigi_valid,L1,DIGI2RAW,L1Reco,RECO,VALIDATION --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --datatier GEN-SIM-DIGI-RECO,DQMIO --beamspot NominalCollision2015 --era Run2_2016 : FAILED - time: date Mon Jun 20 18:51:48 2022-date Mon Jun 20 18:51:43 2022 s - exit: 256
Expand to see more addon errors ...

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38442/30653

@BenjaminRS
Copy link
Contributor

@cms-sw/l1-l2 @cbotta @BenjaminRS
From L1T side, I assume the follow up PR will come following the review, i.e. naming scheme.
Do you need to do something else on you side for this PR?

Hi Phat - what I understand from Andrea's comment regarding the TrackerMuon; this is directed to Thiago and changing the HLT filter interface. Therefore I do not believe anything else is needed from L1T for this PR, and Cécile and Enrique will make a follow up PR for the style formatting.

@cecilecaillol
Copy link
Contributor Author

We will follow up with another PR addressing the minor comments like naming scheme. We are not planning to add anything else to this PR, this is what we need for our MC production. Thanks everyone for the detailed reviews and comments!

@cecilecaillol
Copy link
Contributor Author

+l1

@Martin-Grunewald
Copy link
Contributor

+hlt
(with all the caveats expressed above)

@AdrianoDee
Copy link
Contributor

+upgrade

  • switching Phase2 L1T to emulation (removing simulation);
  • adapting Phase2 HLT accordingly to use emulated objects as inputs;
  • adapting Phase2 L1T DQM accordingly;
  • few open points to be reviewed in a following PR.

@jpata
Copy link
Contributor

jpata commented Aug 25, 2022

+reconstruction

  • the reco part is trivial

@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 perrotta mentioned this pull request Aug 25, 2022
Closed
6 tasks
@perrotta
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 3e2907f into cms-sw:master Aug 25, 2022
@smuzaffar smuzaffar mentioned this pull request Aug 26, 2022
Closed
@perrotta perrotta mentioned this pull request Aug 26, 2022
Merged
@perrotta
Copy link
Contributor

perrotta commented Aug 26, 2022 •
edited
Loading

@cms-sw/l1-l2 @cms-sw/dqm-l2 please notice that, besides the issue with fwlite that is fixed by @smuzaffar with #39213, after the merging of this PR we are noticing spurios differences in the efficiencies for Phase2 muons in DQM in the PR tests, see e.g. #39209 and #39193
This is also something that needs to be fixed!

@perrotta
Copy link
Contributor

@cms-sw/l1-l2 @cms-sw/dqm-l2 please notice that, besides the issue with fwlite that is fixed by @smuzaffar with #39213, after the merging of this PR we are noticing spurios differences in the efficiencies for Phase2 muons in DQM in the PR tests, see e.g. #39209 and #39193
This is also something that needs to be fixed!

Added to issue #39194

@cmsbuild cmsbuild mentioned this pull request Aug 26, 2022
Merged
@makortel makortel mentioned this pull request Aug 29, 2022
Closed
@perrotta perrotta mentioned this pull request Sep 1, 2022
Merged
@makortel makortel mentioned this pull request Sep 2, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@perrotta perrotta perrotta left review comments

@fwyzard fwyzard fwyzard left review comments

@Martin-Grunewald Martin-Grunewald Martin-Grunewald left review comments

@srimanob srimanob srimanob left review comments

@trtomei trtomei trtomei left review comments

@missirol missirol missirol left review comments

@BenjaminRS BenjaminRS BenjaminRS left review comments

@AdrianoDee AdrianoDee AdrianoDee left review comments

Assignees
No one assigned
Labels
code-checks-approved dqm-approved fully-signed hlt-approved l1-approved orp-approved reconstruction-approved requires-external tests-approved upgrade-approved
Projects
None yet
Milestone
CMSSW_12_5_X
Development

Successfully merging this pull request may close these issues.

None yet