streamline the configuration of TriggerObjectTableProducer #40145

vlimant · 2022-11-24T15:40:03Z

PR description:

the initial design of the configuration for TriggerObjectTableProducer is a bit error prone. This is an attempt at making it simpler. Backward compatibility was kept so as to support the V10 configuration without touching it.

PR validation:

elements of the nano matrix function

vlimant · 2022-11-24T15:42:12Z

@silviodonato your feedback is welcome

cmsbuild · 2022-11-24T15:48:04Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40145/33137

This PR adds an extra 20KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-checks:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40145/33137/code-checks.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40145/33137/code-checks.patch | patch -p1
You can also run scram build code-checks to apply code checks directly

silviodonato · 2022-11-24T18:26:39Z

Thanks @vlimant , it look much much better now. Adding also @cms-sw/hlt-l2

I think that you could further simplify the configuration to support the manual description in expsel . Eg. From

mksel("max(filter('hltL3fL1sMu*L3Filtered50*'),filter('hltL3fL1sMu*TkFiltered50*'))","1mu (Mu50)")

to

expsel("hltL3fL1sMu*L3Filtered50* OR hltL3fL1sMu*TkFiltered50*","1mu (Mu50)")

In addition, I would prefer to pass a list ["hltL3fL1sMu*L3Filtered50*","hltL3fL1sMu*TkFiltered50*"] instead of "hltL3fL1sMu*L3Filtered50* OR hltL3fL1sMu*TkFiltered50*" (even for a single filter), but it is just matter of taste.

You might even consider to pass everything as a map read by a single function:

qualityBits = cms.VPSet(expsel({
    "L2p5 pixel iso":["hltL2TauIsoFilter"],
    "1mu (Mu50)": ["hltL3fL1sMu*L3Filtered50*","hltL3fL1sMu*TkFiltered50*"]
    "hltEG40HEFilter":["hltEG40HEFilter"],
    "hltEG90HEFilter":["hltEG90HEFilter"],
}))

I see that there are only two "non-standard" formulas:

mksel("min(filter('hltEle32L1DoubleEGWPTightGsfTrackIsoFilter'),filter('hltEGL1SingleEGOrFilter'))","1e (32_L1DoubleEG_AND_L1SingleEGOr)"),
mksel("path('HLT_Ele*PFJet*')","1e (PFJet)"),

I think that both cases are a kind of "abuse" of the NanoAOD trigger object tool:

min(filter(...),filter(...)) is simply the AND of two trigger filters, which I think should be kept separated (even if we pay the price of an additional bit)
'HLT_Ele*PFJet*' this can be clearly obtained at analysis level, and then it is a "wasted" bit.

I would stop supporting this kind of non-standard trigger objects formulas.

Btw. Shall this code be compatible to both Run-2 and Run-3, or only Run-3?

vlimant · 2022-11-25T11:00:30Z

Thanks @silviodonato ; I made a few changes to allow for [] (means OR) or () (means AND) or full expression.
I don't find the top level function practical enough, and would live it to single entry configuration.

That code / configuration does not address yet run II/III issue

cmsbuild · 2022-11-25T11:05:49Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40145/33143

ERROR: Build errors found during clang-tidy run.

PhysicsTools/NanoAOD/plugins/TriggerObjectTableProducer.cc:119:36: error: reference to non-static member function must be called; did you mean to call it with no arguments? [clang-diagnostic-error]
        if (!qualityBitsFunc.str().empty) {
                                   ^
                                        ()
--
gmake: *** [config/SCRAM/GMake/Makefile.coderules:129: code-checks] Error 2
gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 2

cmsbuild · 2022-11-25T11:37:19Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40145/33144

This PR adds an extra 24KB to repository

cmsbuild · 2022-11-25T11:37:45Z

A new Pull Request was created by @vlimant (vlimant) for master.

It involves the following packages:

PhysicsTools/NanoAOD (xpog)

@cmsbuild, @swertz, @vlimant can you please review it and eventually sign? Thanks.
@gpetruc 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

…ct matching

vlimant · 2022-11-29T10:35:59Z

please test

cmsbuild · 2022-11-29T10:41:43Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40145/33182

This PR adds an extra 20KB to repository

cmsbuild · 2022-11-29T10:42:13Z

Pull request #40145 was updated. @swertz, @vlimant can you please check and sign again.

cmsbuild · 2022-11-29T14:35:40Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f64bb5/29316/summary.html
COMMIT: b753024
CMSSW: CMSSW_13_0_X_2022-11-28-2300/el8_amd64_gcc11
Additional Tests: NANO
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40145/29316/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: 23 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3421159
DQMHistoTests: Total failures: 46
DQMHistoTests: Total nulls: 53
DQMHistoTests: Total successes: 3421038
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 206 log files, 48 edm output root files, 48 DQM output files
TriggerResults: no differences found

NANO Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 132 differences found in the comparisons
DQMHistoTests: Total files compared: 15
DQMHistoTests: Total histograms compared: 14933
DQMHistoTests: Total failures: 240
DQMHistoTests: Total nulls: 98
DQMHistoTests: Total successes: 14595
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 14 files compared)
Checked 31 log files, 0 edm output root files, 15 DQM output files

Nano size comparison Summary:

Sample	kb/ev	ref kb/ev	diff kb/ev	ev/s/thd	ref ev/s/thd	diff rate	mem/thd	ref mem/thd
2500.31	2.213	2.212	0.000 ( +0.0% )	9.33	9.33	-0.0%	1.548	1.570
2500.311	2.334	2.333	0.001 ( +0.0% )	8.88	9.01	-1.4%	1.920	1.942
2500.312	2.288	2.287	0.001 ( +0.0% )	9.07	9.08	-0.1%	1.908	1.950
2500.33	1.103	1.103	-0.000 ( -0.0% )	20.79	21.05	-1.2%	1.681	1.682
2500.331	1.403	1.402	0.000 ( +0.0% )	15.19	15.53	-2.2%	1.844	1.844
2500.332	1.336	1.335	0.001 ( +0.0% )	17.19	17.38	-1.1%	1.785	1.787
2500.4	2.109	2.107	0.001 ( +0.1% )	10.07	10.08	-0.2%	1.447	1.483
2500.401	2.136	2.135	0.000 ( +0.0% )	10.06	10.10	-0.4%	1.262	1.289
2500.5	0.301	0.301	0.000 ( +0.0% )	51.30	53.73	-4.5%	1.189	1.214
2500.501	0.301	0.301	0.000 ( +0.0% )	51.54	52.70	-2.2%	1.207	1.233
2500.51	1.075	1.074	0.000 ( +0.0% )	27.52	28.02	-1.8%	1.448	1.470
2500.511	1.107	1.107	0.000 ( +0.0% )	26.96	27.09	-0.5%	1.481	1.504
2500.6	1.413	1.413	0.000 ( +0.0% )	22.86	23.01	-0.6%	1.145	1.175
2500.601	1.445	1.444	0.001 ( +0.1% )	22.53	22.89	-1.6%	1.174	1.202

vlimant · 2022-11-29T15:30:39Z

+1

cmsbuild · 2022-11-29T15:31:07Z

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

vlimant · 2022-11-29T15:40:12Z

backport to 12.6 in #40173

rappoccio · 2022-11-29T20:59:11Z

+1

streamline the configuration of TriggerObjectTableProducer

8bb8846

cmsbuild added this to the CMSSW_13_0_X milestone Nov 24, 2022

cmsbuild added code-checks-pending orp-pending pending-signatures tests-pending xpog-pending labels Nov 24, 2022

cmsbuild added code-checks-rejected and removed code-checks-pending labels Nov 24, 2022

vlimant added 2 commits November 25, 2022 11:52

code check

c96ca2c

further stream line and flexibility of configuration

523d183

cmsbuild added code-checks-pending and removed code-checks-rejected labels Nov 25, 2022

cmsbuild added code-checks-rejected and removed code-checks-pending labels Nov 25, 2022

typo

151405f

cmsbuild added code-checks-pending and removed code-checks-rejected labels Nov 25, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 25, 2022

economy of evaluating sel.qualityBits. remove break statement in obje…

8197995

…ct matching

cmsbuild added code-checks-pending and removed code-checks-approved labels Nov 25, 2022

cmsbuild added code-checks-pending tests-pending and removed code-checks-approved labels Nov 29, 2022

cmsbuild added tests-started and removed tests-pending labels Nov 29, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 29, 2022

cmsbuild mentioned this pull request Nov 29, 2022

Nano: fix and additions to collection matching, remove cross-cleaning #40162

Merged

swertz mentioned this pull request Nov 29, 2022

NanoDQM: remove consumesMany, plot bits in bitsets #40169

Merged

vlimant mentioned this pull request Nov 29, 2022

[12_6_X] TriggerObjectTableProducer configuration and bug fixes #40173

Merged

cmsbuild added tests-approved and removed tests-started labels Nov 29, 2022

cmsbuild added fully-signed xpog-approved and removed pending-signatures xpog-pending labels Nov 29, 2022

cmsbuild added orp-approved and removed orp-pending labels Nov 29, 2022

cmsbuild merged commit 5a1b1c2 into cms-sw:master Nov 29, 2022

This was referenced Nov 30, 2022

[PPC64LE][LTO] Disable LTO for ppc64le cms-sw/cmsdist#8208

Merged

[G4VECGEOM] Update clhep to 2.4.6.0, vecgeom to 1.2.1 cms-sw/cmsdist#8211

Merged

vlimant deleted the VPSet_4_TriggerObject branch December 4, 2022 11:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

streamline the configuration of TriggerObjectTableProducer #40145

streamline the configuration of TriggerObjectTableProducer #40145

vlimant commented Nov 24, 2022

vlimant commented Nov 24, 2022

cmsbuild commented Nov 24, 2022

silviodonato commented Nov 24, 2022

vlimant commented Nov 25, 2022

cmsbuild commented Nov 25, 2022

cmsbuild commented Nov 25, 2022

cmsbuild commented Nov 25, 2022

vlimant commented Nov 29, 2022

cmsbuild commented Nov 29, 2022

cmsbuild commented Nov 29, 2022

cmsbuild commented Nov 29, 2022

vlimant commented Nov 29, 2022

cmsbuild commented Nov 29, 2022

vlimant commented Nov 29, 2022

rappoccio commented Nov 29, 2022

streamline the configuration of TriggerObjectTableProducer #40145

streamline the configuration of TriggerObjectTableProducer #40145

Conversation

vlimant commented Nov 24, 2022

PR description:

PR validation:

vlimant commented Nov 24, 2022

cmsbuild commented Nov 24, 2022

silviodonato commented Nov 24, 2022

vlimant commented Nov 25, 2022

cmsbuild commented Nov 25, 2022

cmsbuild commented Nov 25, 2022

cmsbuild commented Nov 25, 2022

vlimant commented Nov 29, 2022

cmsbuild commented Nov 29, 2022

cmsbuild commented Nov 29, 2022

cmsbuild commented Nov 29, 2022

Comparison Summary

NANO Comparison Summary

vlimant commented Nov 29, 2022

cmsbuild commented Nov 29, 2022

vlimant commented Nov 29, 2022

rappoccio commented Nov 29, 2022