L1T: remove l1ExtraParticles from stage 2 reco #36916

bundocka · 2022-02-08T18:50:09Z

Removes l1ExtraParticles from Stage 2 L1T reconstruction, which should not be included for Stage 2 (only Stage 1).

Tested with:

runTheMatrix.py --what standard -l 138.4 --command "--conditions 123X_dataRun3_Prompt_dd4hep_Candidate_2022_02_07_15_17_27 -n 1000" -t 8

No longer see error:
"No "L1CaloGeometryRecord" record found in the EventSetup."
since the l1ExtraParticles producer that asks for this record is no longer called in the Stage 2 L1T reco.

tvami · 2022-02-08T18:53:39Z

urgent

tvami · 2022-02-08T18:53:49Z

type bug-fix

tvami · 2022-02-08T18:55:15Z

@cmsbuild , please test

cmsbuild · 2022-02-08T18:56:43Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36916/28225

This PR adds an extra 12KB to repository

cmsbuild · 2022-02-08T18:57:07Z

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

It involves the following packages:

L1Trigger/Configuration (l1)

@epalencia, @rekovic, @cecilecaillol can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @missirol 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

cmsbuild · 2022-02-08T20:14:24Z

-1

Failed Tests: RelVals
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6054b0/22296/summary.html
COMMIT: 07db6f1
CMSSW: CMSSW_12_3_X_2022-02-08-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36916/22296/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

11634.011634.0_TTbar_14TeV+2021+TTbar_14TeV_TuneCP5_GenSim+Digi+RecoNano+HARVESTNano+ALCA/step3_TTbar_14TeV+2021+TTbar_14TeV_TuneCP5_GenSim+Digi+RecoNano+HARVESTNano+ALCA.log
11634.711634.7_TTbar_14TeV+2021_trackingMkFit+TTbar_14TeV_TuneCP5_GenSim+Digi+RecoNano+HARVESTNano/step3_TTbar_14TeV+2021_trackingMkFit+TTbar_14TeV_TuneCP5_GenSim+Digi+RecoNano+HARVESTNano.log
11634.91411634.914_TTbar_14TeV+2021_DDDDB+TTbar_14TeV_TuneCP5_GenSim+Digi+RecoNano+HARVESTNano+ALCA/step3_TTbar_14TeV+2021_DDDDB+TTbar_14TeV_TuneCP5_GenSim+Digi+RecoNano+HARVESTNano+ALCA.log

Expand to see more relval errors ...

tvami · 2022-02-08T20:21:58Z

~~@cms-sw/orp-l2 the issue doesnt actually seem to be coming from this PR, is there a github issue already for what the reason for the failure is?~~

tvami · 2022-02-08T20:26:10Z

Sorry I was probably wrong, I guess the l1extraparticles somehow propagate to the HLTriggerOfflineExotica in the MC @bundocka I think what you need to do is to have the Run-3 era connected to this change, so it will be kept for the MC (if it's really used there -- if not, please change the MC code so it doesnt consume it)

tvami · 2022-02-08T20:34:51Z

L1Trigger/Configuration/python/L1TReco_cff.py

 #
 from Configuration.Eras.Modifier_stage2L1Trigger_cff import stage2L1Trigger
 stage2L1Trigger.toReplaceWith(L1Reco_L1Extra,cms.Sequence())
 stage2L1Trigger.toReplaceWith(L1Reco_L1Extra_L1GtRecord,cms.Sequence())
-stage2L1Trigger.toReplaceWith(L1Reco, cms.Sequence(l1extraParticles))
+stage2L1Trigger.toReplaceWith(L1Reco, cms.Sequence())



Suggested change

stage2L1Trigger.toReplaceWith(L1Reco, cms.Sequence(l1extraParticles))

from Configuration.Eras.Era_Run3_cff import Run3

_L1Reco_forRun3 = L1Reco.copyAndExclude([l1extraParticles])

Run3.toReplaceWith(L1Reco, _L1Reco_forRun3)

wouldnt this work?

srimanob · 2022-02-09T06:45:38Z

Hi @bundocka
Thanks for the fix. Would you mind explaining a bit more in detail? Is the tag being used in Stage-1 (after removing from Stage-2)? And how the tag is used, i.e. why it is very old and no update?

silviodonato · 2022-02-10T09:49:15Z

The DQM code of the HLT needs to be reviewed and updated.
You can remove the line which cause the problem:
https://github.com/cms-sw/cmssw/blob/CMSSW_12_3_X/HLTriggerOffline/Exotica/python/analyses/hltExoticaMETplusTrack_cff.py#L18

(this solves the crash in 12434.0 , I'm currently checking the other wfs)

…Stage 1 L1CaloGeometry record

srimanob · 2022-02-10T12:45:33Z

The DQM code of the HLT needs to be reviewed and updated. You can remove the line which cause the problem: https://github.com/cms-sw/cmssw/blob/CMSSW_12_3_X/HLTriggerOffline/Exotica/python/analyses/hltExoticaMETplusTrack_cff.py#L18

(this solves the crash in 12434.0 , I'm currently checking the other wfs)

Thanks, @silviodonato for reviewing
@tvami Is this PR really urgent? I mean we can survive with an obsolete tag for now, I assume. Or I miss any technical details that make this to be an urgent fix.

srimanob · 2022-02-10T12:49:48Z

@cms-sw/l1-l2
Do we have a place where the development plan of L1T SW is presented? I think it is a good idea that we need to sync a bit. This is not only for Run-3 but also Phase-2 and how do you plan to maintain legacy Run-1, Run-2 in new CMSSW. Is the RECO meeting, or release meeting a good place? Or at any L1T forum.

FYI @dpiparo @perrotta @qliphy

tvami · 2022-02-10T13:09:50Z

@tvami Is this PR really urgent? I mean we can survive with an obsolete tag for now, I assume. Or I miss any technical details that make this to be an urgent fix.

Yes, the original issue was that this was going to be the only non-DD4HEP tag. Since apparently it's "not consumed" while it's consumed (I dont really understand what was happening in hltExoticaValidator -- were we just validating this tag that hasnt changed since 2015, I guess that brings up the point of the validation, anyway going back to the original though) I guess we could survive with having the obsolete tag in.

On the other hand
https://indico.cern.ch/event/1123504/#1-news
slide 9
"Build 12.2.1 today/tomorrow"
I'm also hopeful that this PR will now pass the tests so we can push in the backport in the given timeframe above.

cmsbuild · 2022-02-10T16:57:53Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6054b0/22330/summary.html
COMMIT: 34c6503
CMSSW: CMSSW_12_3_X_2022-02-09-2300/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36916/22330/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: 3 differences found in the comparisons
DQMHistoTests: Total files compared: 46
DQMHistoTests: Total histograms compared: 3764360
DQMHistoTests: Total failures: 4
DQMHistoTests: Total nulls: 12
DQMHistoTests: Total successes: 3764322
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -17.294 KiB( 45 files compared)
DQMHistoSizes: changed ( 10024.0,... ): -0.004 KiB MessageLogger/Errors
DQMHistoSizes: changed ( 10024.0,... ): -0.004 KiB MessageLogger/Warnings
DQMHistoSizes: changed ( 11634.0,... ): -2.845 KiB HLT/Exotica
Checked 193 log files, 42 edm output root files, 46 DQM output files
TriggerResults: found differences in 2 / 45 workflows

srimanob · 2022-02-10T17:25:19Z

Maybe I don't fully understand the issue of cpu-time we spend in the wrong sequence here. Is this documented somewhere? If it is not a big deal, I would propose we do this in the moment that we understand better the situation of L1T development. In addition to Carl's comment, the tag is neither DDD nor DD4hep. It should not be counted as DD4hep migration.

perrotta · 2022-02-10T18:13:11Z

Dear all, we have stopped the building of 12_2_1 because we were asked to wait for this PR.
Please, either proceed with it, or agree among yourselves that this is not actually needed in 12_2, and we'll start building the release without it.

tvami · 2022-02-10T18:17:30Z

My opinion is that since this PR finally converged we should merge it, but it's not under AlCa signature so kindly pinging @cms-sw/dqm-l2 @cms-sw/l1-l2

jfernan2 · 2022-02-10T18:33:09Z

+1
10 MEs removed in folder: HLT/Exotica/METplusTrack

cecilecaillol · 2022-02-10T18:47:04Z

+l1

cmsbuild · 2022-02-10T18:47:26Z

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

rappoccio · 2022-02-10T19:36:28Z

Hi, All, since it's already there, we should merge it. This will allow us to process the entire 0.5B events as a step chain, so this will be much more efficient, if we can get it to work. Considering the geometry (including DD4HEP) migration was also really "due" last fall, it's beyond time to get it in.

perrotta · 2022-02-10T20:09:08Z

+1

@bundocka please prepare a backport PR to 12_2_X

tvami · 2022-02-10T21:08:28Z

+1

@bundocka please prepare a backport PR to 12_2_X

I did it in #36930

srimanob · 2022-02-11T00:11:29Z

Hi, All, since it's already there, we should merge it. This will allow us to process the entire 0.5B events as a step chain, so this will be much more efficient, if we can get it to work. Considering the geometry (including DD4HEP) migration was also really "due" last fall, it's beyond time to get it in.

I don't understand why you say that we can't run stepchain without this PR. This part is under L1Reco where we do stepchain since Run-2. No?

L1T: remove l1ExtraParticles from stage 2 reco

07db6f1

cmsbuild added this to the CMSSW_12_3_X milestone Feb 8, 2022

cmsbuild added code-checks-pending l1-pending orp-pending pending-signatures tests-pending labels Feb 8, 2022

bundocka mentioned this pull request Feb 8, 2022

Obsolete tags PCaloGeometry_Castor_v1_hlt and L1CaloGeometry_CRAFT09_hlt still consumed by code #36806

Closed

cmsbuild added bug-fix urgent labels Feb 8, 2022

cmsbuild added tests-started and removed tests-pending labels Feb 8, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Feb 8, 2022

cmsbuild added tests-rejected and removed tests-started labels Feb 8, 2022

tvami reviewed Feb 8, 2022

View reviewed changes

L1T: set l1MetLabel to none for Stage 2 workflows to prevent calling …

34c6503

…Stage 1 L1CaloGeometry record

cmsbuild removed tests-rejected code-checks-approved labels Feb 10, 2022

cmsbuild added tests-started and removed tests-pending labels Feb 10, 2022

cmsbuild added tests-approved and removed tests-started labels Feb 10, 2022

cmsbuild added dqm-approved and removed dqm-pending labels Feb 10, 2022

cmsbuild added fully-signed l1-approved and removed l1-pending pending-signatures labels Feb 10, 2022

cmsbuild added orp-approved and removed orp-pending labels Feb 10, 2022

cmsbuild merged commit 222afad into cms-sw:master Feb 10, 2022

tvami mentioned this pull request Feb 10, 2022

[122X] L1T: remove l1ExtraParticles from stage 2 reco and its consuption in DQM #36930

Merged

tvami mentioned this pull request Feb 11, 2022

Update Run-3 data and MC GTs with several updates #36940

Merged

borzari mentioned this pull request Mar 10, 2022

Update of Exotica HLT validation monitored paths and removal of l1MET from code #37192

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

L1T: remove l1ExtraParticles from stage 2 reco #36916

L1T: remove l1ExtraParticles from stage 2 reco #36916

bundocka commented Feb 8, 2022

tvami commented Feb 8, 2022

tvami commented Feb 8, 2022

tvami commented Feb 8, 2022

cmsbuild commented Feb 8, 2022

cmsbuild commented Feb 8, 2022

cmsbuild commented Feb 8, 2022

tvami commented Feb 8, 2022 •

edited

tvami commented Feb 8, 2022 •

edited

tvami Feb 8, 2022 •

edited

srimanob commented Feb 9, 2022

silviodonato commented Feb 10, 2022

srimanob commented Feb 10, 2022

srimanob commented Feb 10, 2022

tvami commented Feb 10, 2022 •

edited

cmsbuild commented Feb 10, 2022

srimanob commented Feb 10, 2022

perrotta commented Feb 10, 2022

tvami commented Feb 10, 2022

jfernan2 commented Feb 10, 2022

cecilecaillol commented Feb 10, 2022

cmsbuild commented Feb 10, 2022

rappoccio commented Feb 10, 2022

perrotta commented Feb 10, 2022

tvami commented Feb 10, 2022

srimanob commented Feb 11, 2022

+stage2L1Trigger.toReplaceWith(L1Reco, cms.Sequence(l1extraParticles))
+from Configuration.Eras.Era_Run3_cff import Run3
+_L1Reco_forRun3 = L1Reco.copyAndExclude([l1extraParticles])
+Run3.toReplaceWith(L1Reco, _L1Reco_forRun3)

L1T: remove l1ExtraParticles from stage 2 reco #36916

L1T: remove l1ExtraParticles from stage 2 reco #36916

Conversation

bundocka commented Feb 8, 2022

tvami commented Feb 8, 2022

tvami commented Feb 8, 2022

tvami commented Feb 8, 2022

cmsbuild commented Feb 8, 2022

cmsbuild commented Feb 8, 2022

cmsbuild commented Feb 8, 2022

RelVals

tvami commented Feb 8, 2022 • edited

tvami commented Feb 8, 2022 • edited

tvami Feb 8, 2022 • edited

Choose a reason for hiding this comment

srimanob commented Feb 9, 2022

silviodonato commented Feb 10, 2022

srimanob commented Feb 10, 2022

srimanob commented Feb 10, 2022

tvami commented Feb 10, 2022 • edited

cmsbuild commented Feb 10, 2022

Comparison Summary

srimanob commented Feb 10, 2022

perrotta commented Feb 10, 2022

tvami commented Feb 10, 2022

jfernan2 commented Feb 10, 2022

cecilecaillol commented Feb 10, 2022

cmsbuild commented Feb 10, 2022

rappoccio commented Feb 10, 2022

perrotta commented Feb 10, 2022

tvami commented Feb 10, 2022

srimanob commented Feb 11, 2022

tvami commented Feb 8, 2022 •

edited

tvami commented Feb 8, 2022 •

edited

tvami Feb 8, 2022 •

edited

tvami commented Feb 10, 2022 •

edited