PPS: AlCaReco producer, content filters and testing files #36273

grzanka · 2021-11-28T21:22:53Z

PR description:

This PR adds a new AlCaReco producer to be used in PCL workflows for the PPS subsystem.

PR validation:

This PR was tested with Tests in Calibration/PPSAlCaRecoProducer/test.
Tests were performed on AOD-like Run2 (2018) data which should have the same format as the dataset required by PCL.

if this PR is a backport please specify the original PR and why you need to backport that PR:

N/A

FYI @fabferro @AndreaBellora @jan-kaspar @MatiXOfficial @ChrisMisan

cmsbuild · 2021-11-28T21:29:50Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36273/26962

This PR adds an extra 40KB to repository

cmsbuild · 2021-11-28T21:30:14Z

A new Pull Request was created by @grzanka (Leszek Grzanka) for master.

It involves the following packages:

Calibration/PPSAlCaRecoProducer (****)

The following packages do not have a category, yet:

Calibration/PPSAlCaRecoProducer
Please create a PR for https://github.com/cms-sw/cms-bot/blob/master/categories_map.py to assign category

@cmsbuild can you please review it and eventually sign? Thanks.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

perrotta · 2021-12-01T10:44:59Z

assign alca

cmsbuild · 2021-12-01T10:45:26Z

Pull request #36273 was updated. @cmsbuild, @malbouis, @tvami, @yuanchao, @francescobrivio can you please check and sign again.

tvami · 2021-12-01T14:42:10Z

assign alca

Oh so this PR was submitted 3 days ago? @perrotta @qliphy I think it's actually something that we need in 12_2_X, let me check with PPS

tvami · 2021-12-01T14:42:20Z

@cmsbuild , please test

malbouis · 2021-12-01T15:00:15Z

A first general question about the integration plan. Is this new AlCaReco producer to be used for the already existing PCL workflows? Or would it be only for the AlCa Stream at a first moment?

Calibration/PPSAlCaRecoProducer/python/PPSAlCaRecoProducer_cff.py

cmsbuild · 2021-12-01T17:42:00Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-92e153/20906/summary.html
COMMIT: e6ea381
CMSSW: CMSSW_12_2_X_2021-12-01-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36273/20906/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: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 41
DQMHistoTests: Total histograms compared: 3041955
DQMHistoTests: Total failures: 5
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3041927
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 40 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 175 log files, 37 edm output root files, 41 DQM output files
TriggerResults: no differences found

malbouis · 2021-12-01T19:45:55Z

@grzanka I think you also have to include this AlcaReco producer in https://github.com/cms-sw/cmssw/blob/master/Configuration/StandardSequences/python/AlCaRecoStreams_cff.py

Calibration/PPSAlCaRecoProducer/python/PPSAlCaRecoProducer_cff.py

tvami · 2021-12-01T22:00:53Z

assign alca

Oh so this PR was submitted 3 days ago? @perrotta @qliphy I think it's actually something that we need in 12_2_X, let me check with PPS

After liaising with PPS: we don't need to have this in 12_2_X

grzanka · 2021-12-02T14:20:30Z

assign alca

Oh so this PR was submitted 3 days ago? @perrotta @qliphy I think it's actually something that we need in 12_2_X, let me check with PPS

After liaising with PPS: we don't need to have this in 12_2_X

I confirm, we don't need it at this moment for 12_2_X, as we don't have now the raw files (ALCARAW) on which this producer can be tested.
We only did some tests on raw data from Run2 (as in Calibration/PPSAlCaRecoProducer/test which is included in this PR).

tvami · 2021-12-06T19:50:53Z

Hi @grzanka have you had the chance to check #36273 (comment) ?

tvami · 2022-02-15T16:17:34Z

unhold

cmsbuild · 2022-02-15T17:36:29Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-92e153/22433/summary.html
COMMIT: ead8252
CMSSW: CMSSW_12_3_X_2022-02-14-2300/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36273/22433/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: 46
DQMHistoTests: Total histograms compared: 3764435
DQMHistoTests: Total failures: 13
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3764399
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.004 KiB( 45 files compared)
DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
Checked 193 log files, 42 edm output root files, 46 DQM output files
TriggerResults: no differences found

mmusich · 2022-02-15T17:43:17Z

Calibration/PPSAlCaRecoProducer/test/test_prompt_PPSAlCaReco_output.py

+process.hltAOD.triggerResults = cms.InputTag("TriggerResults","","HLTX")
+process.hltAOD.triggerEvent   = cms.InputTag("hltTriggerSummaryAOD","","HLTX")
+process.hltAOD.triggerName = cms.string("HLT_PPSMaxTracksPerRP4_v1")
+process.hltAOD.stageL1Trigger = cms.uint32(2)


minor comment: this makes the output of the unit test somewhat verbose: given the urgency perhaps can be followed up later.

Good point - it didn't look that bad in the terminal... :) We'll get back to it in the next PR.

tvami · 2022-02-15T18:04:56Z

Hi @MatiXOfficial I see

Last active module - label/type: hltBoolEnd/HLTBool [13 out of 0-13 on this path]
 'L3' filter in slot 5 - label/type hltL1sZeroBias/HLTL1TSeed
   0 accepted 'L3' objects found:

in the output of the unit test. I see this 0 accepted 'L3' objects found: several times. Is that expected?

The producer looks fine tho
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-92e153/22433/unitTests/src/Calibration/PPSAlCaRecoProducer/test/testExpressPPSAlCaRecoProducer/testing.log

tvami · 2022-02-15T18:05:49Z

+alca

with the question in PPS: AlCaReco producer, content filters and testing files #36273 (comment)
and a follow-up to fix for verbosity here PPS: AlCaReco producer, content filters and testing files #36273 (review) and getting rid of the fixed HLT menu

MatiXOfficial · 2022-02-15T18:53:21Z

Hi @tvami,

From what I see these messages appear only in the log of testPromptPPSAlCaRecoOutput and the testExpressPPSAlCaRecoOutput is fine. They are produced only if this hltAOD marked by @mmusich is included in the process.

Since there is no rush this time, I'd prefer to postpone this discussion until next week, when @grzanka returns - unless @AndreaBellora knows something more.

tvami · 2022-02-15T18:58:57Z

Ok, @perrotta @qliphy feel free to review I dont have any further comments

perrotta

A few more question and possible fixes, intended for a forthcoming PR which includes all them (possibly within pre6!)

Calibration/PPSAlCaRecoProducer/python/ALCARECOPPSCalMaxTracks_cff.py

perrotta · 2022-02-16T08:01:54Z

Calibration/PPSAlCaRecoProducer/python/ALCARECOPPSCalMaxTracks_cff.py

+
+from RecoPPS.Local.ctppsDiamondRecHits_cfi import ctppsDiamondRecHits as _ctppsDiamondRecHits
+from RecoPPS.Local.ctppsDiamondLocalTracks_cfi import ctppsDiamondLocalTracks as _ctppsDiamondLocalTracks
+ctppsDiamondRecHitsAlCaRecoProducer = _ctppsDiamondRecHits.clone(digiTag = cms.InputTag('ctppsDiamondRawToDigiAlCaRecoProducer','TimingDiamond'))


Suggested change

ctppsDiamondRecHitsAlCaRecoProducer = _ctppsDiamondRecHits.clone(digiTag = cms.InputTag('ctppsDiamondRawToDigiAlCaRecoProducer','TimingDiamond'))

ctppsDiamondRecHitsAlCaRecoProducer = _ctppsDiamondRecHits.clone(digiTag = 'ctppsDiamondRawToDigiAlCaRecoProducer:TimingDiamond')

perrotta · 2022-02-16T08:02:35Z

Calibration/PPSAlCaRecoProducer/python/ALCARECOPPSCalMaxTracks_cff.py

+from RecoPPS.Local.totemTimingRecHits_cfi import totemTimingRecHits as _totemTimingRecHits
+from RecoPPS.Local.diamondSampicLocalTracks_cfi import diamondSampicLocalTracks as _diamondSampicLocalTracks
+
+totemTimingRecHitsAlCaRecoProducer = _totemTimingRecHits.clone(digiTag = cms.InputTag('totemTimingRawToDigiAlCaRecoProducer','TotemTiming'))


Suggested change

totemTimingRecHitsAlCaRecoProducer = _totemTimingRecHits.clone(digiTag = cms.InputTag('totemTimingRawToDigiAlCaRecoProducer','TotemTiming'))

totemTimingRecHitsAlCaRecoProducer = _totemTimingRecHits.clone(digiTag = 'totemTimingRawToDigiAlCaRecoProducer:TotemTiming')

perrotta · 2022-02-16T08:06:41Z

Calibration/PPSAlCaRecoProducer/test/hlt_out_singleRAW.py

+import FWCore.ParameterSet.Config as cms
+
+process = cms.Process( "HLTX" )
+process.load("setup_dev_CMSSW_12_1_0_GRun_V14_cff")


As I said, it is an overkill to dump a whole GRun menu only to access one (or a few) AlCa paths that you need for your test: please move to a lighter menu, and possibly a less obsolete version of it

perrotta · 2022-02-16T08:08:12Z

Calibration/PPSAlCaRecoProducer/test/hlt_out_singleRAW.py

+        'file:RelVal_Raw_GRun_DATA.root',
+    ),
+    inputCommands = cms.untracked.vstring(
+        'keep *'


Normally scary for the event size... but ok, it is a test script...

Note that this is Source's inputCommands, i.e. for dropping some branches on input. Single keep * is redundant though, and for clarity the whole inputCommands PSet could be dropped.

perrotta · 2022-02-16T08:10:22Z

Calibration/PPSAlCaRecoProducer/test/test_express_AlCaRecoProducer.sh

+function die { echo $1: status $2; exit $2; }
+
+customise_commands="process.GlobalTag.toGet = cms.VPSet()\
+\nprocess.GlobalTag.toGet.append(cms.PSet(record = cms.string(\"AlCaRecoTriggerBitsRcd\"),tag =  cms.string(\"AlCaRecoHLTpaths_PPS2022_express_v1\"), connect = cms.string(\"frontier://FrontierProd/CMS_CONDITIONS\")))\


Are these tags still not available in the GT?

They are, but they are added to the online GT, thus valid from 2022, i.e. it wont work on a replay with 2018 data. Everything with PPS will be just so much clearer when they collect their first data this year...

perrotta · 2022-02-16T08:20:51Z

+operations

Amendments for this PR are certainly needed, as listed in
Please proceed with a fix-up PR as soon as pre5 is out

cmsbuild · 2022-02-16T08:21:13Z

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)

perrotta · 2022-02-16T08:22:23Z

+1

Most of the updates are in the test area
Since this is apparently urgently needed, to be tested at P5 with pre5, let have it merged as such: but please follow up as soon as possible with the issues reported in the review, and summarized above

PPSAlCaRecoProducer - minor fixes requested in #36273

cmsbuild added this to the CMSSW_12_2_X milestone Nov 28, 2021

cmsbuild added code-checks-pending new-package-pending orp-pending pending-signatures tests-pending labels Nov 28, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 28, 2021

qliphy mentioned this pull request Nov 29, 2021

Add Calibration/PPSAlCaRecoProducer to the alca category cms-sw/cms-bot#1668

Merged

cmsbuild added alca-pending and removed new-package-pending labels Dec 1, 2021

cmsbuild added tests-started and removed tests-pending labels Dec 1, 2021

malbouis reviewed Dec 1, 2021

View reviewed changes

Calibration/PPSAlCaRecoProducer/python/PPSAlCaRecoProducer_cff.py Outdated Show resolved Hide resolved

cmsbuild added tests-approved and removed tests-started labels Dec 1, 2021

malbouis reviewed Dec 1, 2021

View reviewed changes

Calibration/PPSAlCaRecoProducer/python/PPSAlCaRecoProducer_cff.py Outdated Show resolved Hide resolved

smuzaffar removed this from the CMSSW_12_2_X milestone Dec 6, 2021

cmsbuild removed the hold label Feb 15, 2022

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

mmusich reviewed Feb 15, 2022

View reviewed changes

cmsbuild added alca-approved and removed alca-pending labels Feb 15, 2022

tvami mentioned this pull request Feb 15, 2022

Fix HPTDC timing PCL #36976

Merged

perrotta reviewed Feb 16, 2022

View reviewed changes

cmsbuild added fully-signed operations-approved and removed operations-pending pending-signatures labels Feb 16, 2022

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

cmsbuild merged commit a3446c6 into cms-sw:master Feb 16, 2022

This was referenced Feb 16, 2022

[HGCAL trigger] Migration to new code structure - First pass #36337

Merged

mkFit external integration #36546

Merged

remove mkFit external cms-sw/cmsdist#7528

Merged

This was referenced Feb 23, 2022

PPSAlCaRecoProducer - minor fixes requested in #36273 #37048

Merged

Updated the PPS PCL workers to support the new AlCaRecoProducer #37050

Merged

cmsbuild added a commit that referenced this pull request Feb 23, 2022

Merge pull request #37048 from CTPPS/pps_alcarecoproducer_fixes

9bed8b9

PPSAlCaRecoProducer - minor fixes requested in #36273

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PPS: AlCaReco producer, content filters and testing files #36273

PPS: AlCaReco producer, content filters and testing files #36273

grzanka commented Nov 28, 2021

cmsbuild commented Nov 28, 2021

cmsbuild commented Nov 28, 2021

perrotta commented Dec 1, 2021

cmsbuild commented Dec 1, 2021

tvami commented Dec 1, 2021

tvami commented Dec 1, 2021

malbouis commented Dec 1, 2021

cmsbuild commented Dec 1, 2021

malbouis commented Dec 1, 2021

tvami commented Dec 1, 2021

grzanka commented Dec 2, 2021

tvami commented Dec 6, 2021

tvami commented Feb 15, 2022

cmsbuild commented Feb 15, 2022

mmusich Feb 15, 2022

MatiXOfficial Feb 15, 2022

tvami commented Feb 15, 2022

tvami commented Feb 15, 2022

MatiXOfficial commented Feb 15, 2022

tvami commented Feb 15, 2022

perrotta left a comment

perrotta Feb 16, 2022

perrotta Feb 16, 2022

perrotta Feb 16, 2022

perrotta Feb 16, 2022

makortel Feb 16, 2022

perrotta Feb 16, 2022

tvami Feb 16, 2022

perrotta commented Feb 16, 2022 •

edited

cmsbuild commented Feb 16, 2022

perrotta commented Feb 16, 2022

	ctppsDiamondRecHitsAlCaRecoProducer = _ctppsDiamondRecHits.clone(digiTag = cms.InputTag('ctppsDiamondRawToDigiAlCaRecoProducer','TimingDiamond'))
	ctppsDiamondRecHitsAlCaRecoProducer = _ctppsDiamondRecHits.clone(digiTag = 'ctppsDiamondRawToDigiAlCaRecoProducer:TimingDiamond')

	totemTimingRecHitsAlCaRecoProducer = _totemTimingRecHits.clone(digiTag = cms.InputTag('totemTimingRawToDigiAlCaRecoProducer','TotemTiming'))
	totemTimingRecHitsAlCaRecoProducer = _totemTimingRecHits.clone(digiTag = 'totemTimingRawToDigiAlCaRecoProducer:TotemTiming')

PPS: AlCaReco producer, content filters and testing files #36273

PPS: AlCaReco producer, content filters and testing files #36273

Conversation

grzanka commented Nov 28, 2021

PR description:

PR validation:

if this PR is a backport please specify the original PR and why you need to backport that PR:

cmsbuild commented Nov 28, 2021

cmsbuild commented Nov 28, 2021

perrotta commented Dec 1, 2021

cmsbuild commented Dec 1, 2021

tvami commented Dec 1, 2021

tvami commented Dec 1, 2021

malbouis commented Dec 1, 2021

cmsbuild commented Dec 1, 2021

Comparison Summary

malbouis commented Dec 1, 2021

tvami commented Dec 1, 2021

grzanka commented Dec 2, 2021

tvami commented Dec 6, 2021

tvami commented Feb 15, 2022

cmsbuild commented Feb 15, 2022

Comparison Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tvami commented Feb 15, 2022

tvami commented Feb 15, 2022

MatiXOfficial commented Feb 15, 2022

tvami commented Feb 15, 2022

perrotta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

perrotta commented Feb 16, 2022 • edited

cmsbuild commented Feb 16, 2022

perrotta commented Feb 16, 2022

perrotta commented Feb 16, 2022 •

edited