Fix HLTEcalPhiSymFilter and add spike cleaning option #49852

thomreis · 2026-01-16T15:58:40Z

PR description:

This PR fixes an issue where status flags of non-existing rechits are accessed. This resulted in the number of digis stored in the output collection being non-deterministic.
In addition an option to run the spike cleaning algorithm with potentially different (tighter) settings than in the EcalRecHitProducer was added.
Refactoring of the code improved the execution time by removing calculations for uncalibrated rechits below the amplitude threshold.

PR validation:

With the fix the output digis collection contains the same number of elements every time the code runs.
Adding tighter spike killer thresholds results in fewer digis being stored as expected.
Passed most matrix tests (some failed at the DBS step).

cmsbuild · 2026-01-16T15:59:06Z

cms-bot internal usage

thomreis · 2026-01-16T16:00:18Z

type ecal

cmsbuild · 2026-01-16T16:01:09Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49852/47526

There are other open Pull requests which might conflict with changes you have proposed:
- File HLTrigger/Configuration/python/customizeHLTforCMSSW.py modified in PR(s): Remove legacy CUDA pixel EventSetup modules #49799

thomreis · 2026-01-16T16:01:23Z

please test

cmsbuild · 2026-01-16T16:01:31Z

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

It involves the following packages:

HLTrigger/Configuration (hlt)
HLTrigger/special (hlt)

@Martin-Grunewald, @mmusich can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @ReyerBand, @argiro, @missirol, @mmusich, @rchatter, @thomreis, @wang0jin this is something you requested to watch as well.
@ftenchini, @mandrenguyen, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

mmusich · 2026-01-16T16:03:32Z

Refactoring of the code improved the execution time by removing

@thomreis the PR description is incomplete. Please check, fix.

mmusich · 2026-01-16T16:04:48Z

HLTrigger/special/plugins/HLTEcalPhiSymFilter.cc

+                   true >> (edm::ParameterDescription<edm::InputTag>(
+                                "barrelHitCollection", edm::InputTag("ecalRecHit", "EcalRecHitsEB"), true) and
+                            edm::ParameterDescription<edm::InputTag>(
+                                "endcapHitCollection", edm::InputTag("ecalRecHit", "EcalRecHitsEE"), true)));


not sure confDB plays nice with this. @Martin-Grunewald FYI

mmusich · 2026-01-16T16:04:57Z

HLTrigger/special/plugins/HLTEcalPhiSymFilter.cc

+    psd0.add<double>("e4e1_b_endcap", -0.0125);
+    psd0.add<double>("e4e1_a_endcap", 0.02);
+    psd0.add<double>("cThreshold_double", 10);
+    desc.ifValue(edm::ParameterDescription<bool>("cleanReco", false, true),


Martin-Grunewald · 2026-01-16T16:05:25Z

HLTrigger/special/plugins/HLTEcalPhiSymFilter.cc

-  desc.add<edm::InputTag>("endcapHitCollection", edm::InputTag("ecalRecHit", "EcalRecHitsEE"));
  desc.add<unsigned int>("statusThreshold", 3);
-  desc.add<bool>("useRecoFlag", false);
+  desc.ifValue(edm::ParameterDescription<bool>("useRecoFlag", false, true),


This does not work with ConfDB!

Martin-Grunewald · 2026-01-16T16:06:17Z

HLTrigger/special/plugins/HLTEcalPhiSymFilter.cc

+    psd0.add<double>("e4e1_b_endcap", -0.0125);
+    psd0.add<double>("e4e1_a_endcap", 0.02);
+    psd0.add<double>("cThreshold_double", 10);
+    desc.ifValue(edm::ParameterDescription<bool>("cleanReco", false, true),


This does not work with ConfDB.

Martin-Grunewald · 2026-01-16T16:08:28Z

Sorry, what do you want to achieve here?

For ConfDb, there is a unique set of top-level module parameters enforced. You can not have variants!
However, inside a PSet parameter, you can have different parameters. Could you restructure this way?

cmsbuild · 2026-01-16T17:40:42Z

-1

Failed Tests: UnitTests RelVals RelVals-INPUT AddOn
Size: This PR adds an extra 32KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a23385/50687/summary.html
COMMIT: bd3add5
CMSSW: CMSSW_16_1_X_2026-01-16-1100/el8_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/49852/50687/install.sh to create a dev area with all the needed externals and cmssw changes.

Failed Unit Tests

I found 3 errors in the following unit tests:

---> test test_HIonFullOutput had ERRORS
---> test test_SpecialFullOutput had ERRORS
---> test test_GRunFullOutput had ERRORS

Failed RelVals

18634.0

----- Begin Fatal Exception 16-Jan-2026 17:34:02 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=HLTEcalPhiSymFilter label='hltEcalPhiSymFilter'
Exception Message:
Illegal parameters found in configuration.  The parameters are named:
 'barrelHitCollection'
 'endcapHitCollection'
You could be trying to use parameter names that are not
allowed for this plugin, or they could be misspelled, or these parameters
need to be defined in the fillDescriptions() method of the plugin.
----- End Fatal Exception -------------------------------------------------

18434.0

----- Begin Fatal Exception 16-Jan-2026 17:33:58 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=HLTEcalPhiSymFilter label='hltEcalPhiSymFilter'
Exception Message:
Illegal parameters found in configuration.  The parameters are named:
 'barrelHitCollection'
 'endcapHitCollection'
You could be trying to use parameter names that are not
allowed for this plugin, or they could be misspelled, or these parameters
need to be defined in the fillDescriptions() method of the plugin.
----- End Fatal Exception -------------------------------------------------

2025.0000001

----- Begin Fatal Exception 16-Jan-2026 17:22:53 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=HLTEcalPhiSymFilter label='hltEcalPhiSymFilter'
Exception Message:
Illegal parameters found in configuration.  The parameters are named:
 'barrelHitCollection'
 'endcapHitCollection'
You could be trying to use parameter names that are not
allowed for this plugin, or they could be misspelled, or these parameters
need to be defined in the fillDescriptions() method of the plugin.
----- End Fatal Exception -------------------------------------------------

Expand to see more relval errors ...

2025.0010001

Failed RelVals-INPUT

2025.00200012025.0020001_RunEGamma02025D_10k/step2_RunEGamma02025D_10k.log
142.0142.0_RunHIPhysicsRawPrime2023A/step2_RunHIPhysicsRawPrime2023A.log
2025.00000012025.0000001_RunZeroBias2025B_10k/step2_RunZeroBias2025B_10k.log

Expand to see more relval errors ...

2025.0010001

Failed AddOn Tests

hlt_mc_GRun

----- Begin Fatal Exception 16-Jan-2026 17:17:32 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=HLTEcalPhiSymFilter label='hltEcalPhiSymFilter'
Exception Message:
Illegal parameters found in configuration.  The parameters are named:
 'barrelHitCollection'
 'endcapHitCollection'
You could be trying to use parameter names that are not
allowed for this plugin, or they could be misspelled, or these parameters
need to be defined in the fillDescriptions() method of the plugin.
----- End Fatal Exception -------------------------------------------------

hlt_mc_GRun

----- Begin Fatal Exception 16-Jan-2026 17:17:56 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=HLTEcalPhiSymFilter label='hltEcalPhiSymFilter'
Exception Message:
Illegal parameters found in configuration.  The parameters are named:
 'barrelHitCollection'
 'endcapHitCollection'
You could be trying to use parameter names that are not
allowed for this plugin, or they could be misspelled, or these parameters
need to be defined in the fillDescriptions() method of the plugin.
----- End Fatal Exception -------------------------------------------------

hlt_mc_HIon

----- Begin Fatal Exception 16-Jan-2026 17:17:32 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=HLTEcalPhiSymFilter label='hltEcalPhiSymFilter'
Exception Message:
Illegal parameters found in configuration.  The parameters are named:
 'barrelHitCollection'
 'endcapHitCollection'
You could be trying to use parameter names that are not
allowed for this plugin, or they could be misspelled, or these parameters
need to be defined in the fillDescriptions() method of the plugin.
----- End Fatal Exception -------------------------------------------------

Expand to see more addon errors ...

cmsbuild · 2026-01-19T11:13:17Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49852/47550

ERROR: Unable to merge PR.

See log https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49852/47550/cms-checkout-topic.log

cmsbuild · 2026-01-19T12:36:12Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49852/47552

cmsbuild · 2026-01-19T12:36:33Z

Pull request #49852 was updated. @Martin-Grunewald, @cmsbuild, @mmusich can you please check and sign again.

mmusich · 2026-01-19T12:37:15Z

@cmsbuild, please test

cmsbuild · 2026-01-19T14:52:04Z

+1

Size: This PR adds an extra 28KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a23385/50725/summary.html
COMMIT: 5c71693
CMSSW: CMSSW_16_1_X_2026-01-19-1100/el8_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/49852/50725/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: 12 differences found in the comparisons
DQMHistoTests: Total files compared: 52
DQMHistoTests: Total histograms compared: 4025536
DQMHistoTests: Total failures: 68
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 4025448
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 51 files compared)
Checked 222 log files, 193 edm output root files, 52 DQM output files
TriggerResults: no differences found

mmusich · 2026-01-20T13:51:38Z

+hlt

cmsbuild · 2026-01-20T13:52:04Z

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

mandrenguyen · 2026-01-21T09:29:40Z

+1

Martin-Grunewald · 2026-01-23T15:38:47Z

HLTrigger/Configuration/python/customizeHLTforCMSSW.py

+            setattr(prod, 'cleanReco', cms.bool(False))
+        if not hasattr(prod, 'cleaningConfig'):
+            from RecoLocalCalo.EcalRecAlgos.ecalCleaningAlgo import cleaningAlgoConfig
+            setattr(prod, 'cleaningConfig', cleaningAlgoConfig)


The cleaningConfig imported here is arsing from a ~12 year old file: https://github.com/cms-sw/cmssw/blob/master/RecoLocalCalo/EcalRecAlgos/python/ecalCleaningAlgo.py while what you code in the fillDescriptions method down below seems way more up-to-date (and as used elsewhere). So I assume we should use the latter.
Also, since fillDescriptions provides both cleanReco as well as cleaningConfig, there should not be the need for any of the customisation above, correct? Of course, the PRs are already merged, but just to clarify on the settings!

If the missing parameters in existing menus are filled in from the default values in the fillDescriptions then this customization not needed indeed.
The cleaning parameters from fillDescriptions are the up to date ones and the ones from cleaningAlgoConfig were just meant as placeholders under the assumption that cleanReco would be False whenever cleaningConfig was not present.

Thanks for the clarification!

indeed

diff --git a/HLTrigger/Configuration/python/customizeHLTforCMSSW.py b/HLTrigger/Configuration/python/customizeHLTforCMSSW.py index ac5d2bcd684..b110405746b 100644 --- a/HLTrigger/Configuration/python/customizeHLTforCMSSW.py +++ b/HLTrigger/Configuration/python/customizeHLTforCMSSW.py @@ -228,6 +228,6 @@ def customizeHLTforCMSSW(process, menuType="GRun"): # process = customizeHLTfor49436(process) process = customizeHLTfor49799(process) - process = customizeHLTfor49852(process) + #process = customizeHLTfor49852(process) return process

passes addOnTests.py just fine.

Of course, the PRs are already merged, but just to clarify on the settings!

shall we remove it now?

Can be removed yes.

And the backport #49924

…zation Remove HLT customization for #49852

cmsbuild added this to the CMSSW_16_1_X milestone Jan 16, 2026

cmsbuild added hlt-pending pending-signatures tests-pending orp-pending code-checks-pending labels Jan 16, 2026

cmsbuild added the ecal label Jan 16, 2026

cmsbuild added tests-started code-checks-approved and removed tests-pending code-checks-pending labels Jan 16, 2026

mmusich reviewed Jan 16, 2026

View reviewed changes

Martin-Grunewald reviewed Jan 16, 2026

View reviewed changes

cmsbuild added tests-rejected tests-pending code-checks-pending and removed tests-started tests-rejected code-checks-approved labels Jan 16, 2026

cmsbuild added the code-checks-pending label Jan 19, 2026

cmsbuild added code-checks-approved and removed code-checks-pending labels Jan 19, 2026

cmsbuild added tests-started and removed tests-pending labels Jan 19, 2026

cmsbuild added tests-approved and removed tests-started labels Jan 19, 2026

thomreis mentioned this pull request Jan 19, 2026

Fix HLTEcalPhiSymFilter and add spike cleaning option - 160X #49871

Merged

cmsbuild added hlt-approved fully-signed and removed hlt-pending pending-signatures labels Jan 20, 2026

cmsbuild added orp-approved and removed orp-pending labels Jan 21, 2026

cmsbuild merged commit e37014e into cms-sw:master Jan 21, 2026
10 checks passed

thomreis deleted the phisymfilter-swisscross branch January 21, 2026 09:33

cmsbuild mentioned this pull request Jan 21, 2026

Run the ONNX test on GPU only if CUDA is available #49883

Merged

Martin-Grunewald reviewed Jan 23, 2026

View reviewed changes

cmsbuild mentioned this pull request Jan 23, 2026

Merging CMSSW_16_1 into CMSSW_16_1_EVOLUTION_X #49910

Merged

thomreis added a commit to thomreis/cmssw that referenced this pull request Jan 23, 2026

Remove HLT customization for cms-sw#49852.

a926873

This was referenced Jan 23, 2026

Remove HLT customization for #49852 #49923

Merged

Remove HLT customization for #49871 #49924

Merged

cmsbuild added a commit that referenced this pull request Jan 24, 2026

Merge pull request #49923 from thomreis/remove-ecalphisym-hlt-customi…

c9f6077

…zation Remove HLT customization for #49852

Fix HLTEcalPhiSymFilter and add spike cleaning option #49852

Fix HLTEcalPhiSymFilter and add spike cleaning option #49852

Conversation

thomreis commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR description:

PR validation:

Uh oh!

cmsbuild commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thomreis commented Jan 16, 2026

Uh oh!

cmsbuild commented Jan 16, 2026

Uh oh!

thomreis commented Jan 16, 2026

Uh oh!

cmsbuild commented Jan 16, 2026

Uh oh!

mmusich commented Jan 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Martin-Grunewald commented Jan 16, 2026

Uh oh!

cmsbuild commented Jan 16, 2026

Failed Unit Tests

Failed RelVals

Failed RelVals-INPUT

Failed AddOn Tests

Uh oh!

cmsbuild commented Jan 19, 2026

Uh oh!

cmsbuild commented Jan 19, 2026

Uh oh!

cmsbuild commented Jan 19, 2026

Uh oh!

mmusich commented Jan 19, 2026

Uh oh!

cmsbuild commented Jan 19, 2026

Comparison Summary

Uh oh!

mmusich commented Jan 20, 2026

Uh oh!

cmsbuild commented Jan 20, 2026

Uh oh!

mandrenguyen commented Jan 21, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

thomreis commented Jan 16, 2026 •

edited

Loading

cmsbuild commented Jan 16, 2026 •

edited

Loading