Make duplicate branch check work properly with SwitchProducer that has EDAlias case #40136

makortel · 2022-11-23T02:24:42Z

PR description:

#40110 demonstrated that the OutputModule's duplicate branch check for EDAliases did not work properly when a SwitchProducer had an EDAlias case. The problem was that the SwitchProducer-EDAlias' alias-for BranchID was not inserted into the trueBranchIDToKeptBranchDesc, and therefore the check threw an exception only if the non-SwitchProducer-EDAlias duplicate entry was earlier in the map. This PR fixes the check to use the proper alias-for BranchID for both normal and SwitchProducer EDAliases.

The first commit consolidates some of the SwitchProducer unit test configuration files to reduce copy paste.

Resolves #40110

PR validation:

Framework unit tests run. The example HLT configuration throws now the proper exception.

To reduce copy-paste

…s EDAlias case Earlier check worked only if the non-SwitchProducer branch was in the map earlier.

cmsbuild · 2022-11-23T02:30:40Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40136/33129

This PR adds an extra 32KB to repository

cmsbuild · 2022-11-23T02:31:07Z

A new Pull Request was created by @makortel (Matti Kortelainen) for master.

It involves the following packages:

FWCore/Framework (core)
FWCore/Integration (core)

@cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please review it and eventually sign? Thanks.
@missirol, @wddgit 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

makortel · 2022-11-23T02:42:08Z

@cmsbuild, please test

cmsbuild · 2022-11-23T04:18:31Z

-1

Failed Tests: RelVals RelVals-INPUT AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-420682/29201/summary.html
COMMIT: 3759817
CMSSW: CMSSW_12_6_X_2022-11-22-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/40136/29201/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

139.001

----- Begin Fatal Exception 23-Nov-2022 04:08:03 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
Exception Message:
Duplicate Output Selection Two (or more) equivalent branches have been selected for output.
#1: BranchKey(HBHERecHitsSorted, hltHbherecoLegacy, , reHLT)
#2: BranchKey(HBHERecHitsSorted, hltHbhereco, , reHLT)
Please drop at least one of them.
----- End Fatal Exception -------------------------------------------------

11634.0

----- Begin Fatal Exception 23-Nov-2022 04:21:24 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
Exception Message:
Duplicate Output Selection Two (or more) equivalent branches have been selected for output.
#1: BranchKey(HBHERecHitsSorted, hltHbherecoLegacy, , HLT)
#2: BranchKey(HBHERecHitsSorted, hltHbhereco, , HLT)
Please drop at least one of them.
----- End Fatal Exception -------------------------------------------------

11634.7

----- Begin Fatal Exception 23-Nov-2022 04:21:24 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
Exception Message:
Duplicate Output Selection Two (or more) equivalent branches have been selected for output.
#1: BranchKey(HBHERecHitsSorted, hltHbherecoLegacy, , HLT)
#2: BranchKey(HBHERecHitsSorted, hltHbhereco, , HLT)
Please drop at least one of them.
----- End Fatal Exception -------------------------------------------------

Expand to see more relval errors ...

RelVals-INPUT

139.003139.003_RunHLTPhy2021+RunHLTPhy2021+HLTDR3_2022+RECODR3_reHLT_HLTPhysics_Offline+HARVESTD2021HLTPhy/step2_RunHLTPhy2021+RunHLTPhy2021+HLTDR3_2022+RECODR3_reHLT_HLTPhysics_Offline+HARVESTD2021HLTPhy.log
139.001139.001_RunMinimumBias2021+RunMinimumBias2021+HLTDR3_2022+RECODR3_reHLT_MinBiasOffline+HARVESTD2021MB/step2_RunMinimumBias2021+RunMinimumBias2021+HLTDR3_2022+RECODR3_reHLT_MinBiasOffline+HARVESTD2021MB.log
139.004139.004_RunNoBPTX2021+RunNoBPTX2021+HLTDR3_2022+RECODR3_reHLT_AlCaTkCosmics_Offline+HARVESTDR3/step2_RunNoBPTX2021+RunNoBPTX2021+HLTDR3_2022+RECODR3_reHLT_AlCaTkCosmics_Offline+HARVESTDR3.log

Expand to see more relval errors ...

AddOn Tests

hlt_mc_GRun

----- Begin Fatal Exception 23-Nov-2022 04:12:29 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
Exception Message:
Duplicate Output Selection Two (or more) equivalent branches have been selected for output.
#1: BranchKey(HBHERecHitsSorted, hltHbherecoLegacy, , HLTRECO)
#2: BranchKey(HBHERecHitsSorted, hltHbhereco, , HLTRECO)
Please drop at least one of them.
----- End Fatal Exception -------------------------------------------------

hlt_mc_HIon

----- Begin Fatal Exception 23-Nov-2022 04:10:54 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
Exception Message:
Duplicate Output Selection Two (or more) equivalent branches have been selected for output.
#1: BranchKey(HBHERecHitsSorted, hltHbherecoLegacy, , HLTRECO)
#2: BranchKey(HBHERecHitsSorted, hltHbhereco, , HLTRECO)
Please drop at least one of them.
----- End Fatal Exception -------------------------------------------------

hlt_mc_PRef

----- Begin Fatal Exception 23-Nov-2022 04:10:37 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
Exception Message:
Duplicate Output Selection Two (or more) equivalent branches have been selected for output.
#1: BranchKey(HBHERecHitsSorted, hltHbherecoLegacy, , HLTRECO)
#2: BranchKey(HBHERecHitsSorted, hltHbhereco, , HLTRECO)
Please drop at least one of them.
----- End Fatal Exception -------------------------------------------------

Expand to see more addon errors ...

missirol · 2022-11-23T07:04:48Z

Looks like this will require a change in HLTrigger_EventContent_cff (either to the script that creates that cff, or to the inputs it uses).

cmssw/HLTrigger/Configuration/python/HLTrigger_EventContent_cff.py

Lines 528 to 530 in 08fcc8e

    
           'keep *_hltHbherecoFromGPU_*_*', 
        
           'keep *_hltHbherecoLegacy_*_*', 
        
           'keep *_hltHbhereco_*_*',

(cc: @Martin-Grunewald)

Martin-Grunewald · 2022-11-23T07:28:08Z

Is it enough to drop hltHbhereco but keeping both hltHbherecoLegacy and hltHbherecoFromGPU?
Indeed it does. So instead of the generic hltHbhereco we could add in the affected EventContents used to build the overall HLT event contents only one or both of the two others, but clients would then need to be updated to use those.
These four: hltOutputDQM,hltOutputHLTMonitor,hltOutputHIHLTMonitor,hltOutputHIDQM
have keep *_hltHbhereco_*_* and the non-HI ones are used to build the EventContent.

missirol · 2022-11-23T10:58:55Z

Is it enough to drop hltHbhereco but keeping both hltHbherecoLegacy and hltHbherecoFromGPU?

I think it would solve this problem, but I think hltHbhereco is needed downstream, for example by DQM modules in RelVal wfs (e.g. HcalRecHitsValidation in step3 of wf 11634.0; although that client might just silently not fill histograms if the collection is missing, as opposed to throwing an exception).

On the HLT side:

It looks like we would have to manually drop some of these collections when we make the cff (as opposed to changing online EventContents in ConfDB, where hltHbhereco is required by the DQM stream (and others), and the other two collections are needed in the DQMGPUvsCPU stream). The alternative would be to remove the DQMGPUvsCPU EventContent from HLTDebugRAW/FEVT, but I think we discussed in the past that it should be in there.

More on the sw side:

I don't understand why we didn't see an issue like Segmentation violation in all DQM online clients during HI run #40110 when using FEVTDEBUGHLT files produced so far, where we were keeping both hltHbhereco and hltHbherecoLegacy / hltHbherecoFromGPU, e.g. step2 of wf 11634.0.
For the future, should we live with this limitation, or are improvements possible? I'm thinking about two files of one MC sample, one file produced on CPU and the other on GPU, and a user having to consume hltHbherecoLegacy in one case and hltHbherecoFromGPU in the other (assuming we have to drop hltHbhereco), whereas the user would just consume hltHbhereco instead.

Tagging @fwyzard in case he has opinions/suggestions/clarifications, since this involves HLT and GPUs.

makortel · 2022-11-23T20:42:35Z

I don't understand why we didn't see an issue like Segmentation violation in all DQM online clients during HI run #40110 when using FEVTDEBUGHLT files produced so far, where we were keeping both hltHbhereco and hltHbherecoLegacy / hltHbherecoFromGPU, e.g. step2 of wf 11634.0.

This is a good question. I see the step3 of workflow 11634.0 consumes only hltHbhereco. But also a specific test reading both hltHbhereco and hltHbherecoLegacy from a ROOT file succeeded. It could be that there is something in the ROOT output/input that allows the setup to work in some situations.

For the future, should we live with this limitation, or are improvements possible? I'm thinking about two files of one MC sample, one file produced on CPU and the other on GPU, and a user having to consume hltHbherecoLegacy in one case and hltHbherecoFromGPU in the other (assuming we have to drop hltHbhereco), whereas the user would just consume hltHbhereco instead.

This limitation is intended behavior (since the introduction of EDAlias a decade ago), because otherwise we'd get to issues like seen in #40110. A high-level premise has been that it doesn't make sense to store both alias and alias-for products (what would be the point of the alias?).

It seems to me we're mixing two different use cases into one

Portable: can be run with or without a GPU, output product has always the same label (hltHbhereco), downstream consumers don't (have to) care if the product was produced with or without a GPU
Non-portable: must be run with a GPU, separate module labels for the CPU and GPU modules (hltHbherecoLegacy vs hltHbherecoFromGPU), consumers must be explicitly aware

I think it would be better to deal with these use cases separately with separate files.

missirol · 2022-11-23T21:47:30Z

I think it would be better to deal with these use cases separately with separate files.

Thanks for clarifying. I would say this goes in the direction of keeping hltHbhereco in the existing tiers (e.g. FEVTDEBUGHLT), removing the DQMGPUvsCPU EventContent from them. If so, the update of HLT_EventContent_cff to unstuck this PR can be done by the end of the week.

We would then have to come up with a separate offline event content for wfs dedicated to HLT GPU-vs-CPU comparisons. In a way, this is similar to what is already done online, where different clients are fed by different streams (e.g. DQM with hltHbhereco, and DQMGPUvsCPU with the explicit CPU/GPU products).

cmsbuild · 2022-11-26T00:44:10Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-420682/29258/summary.html
COMMIT: 3759817
CMSSW: CMSSW_12_6_X_2022-11-25-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/40136/29258/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: 2 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3417232
DQMHistoTests: Total failures: 24
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3417186
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

missirol · 2022-11-26T11:22:36Z

I see the step3 of workflow 11634.0 consumes only hltHbhereco.

Just for the record, step3 of wf 11634.0 also runs a GlobalRecHitsAnalyzer which consumed both hltHbhereco and hltHbherecoLegacy (on CPU), see #40155 (comment).

makortel · 2022-11-28T16:10:22Z

I see the step3 of workflow 11634.0 consumes only hltHbhereco.

Just for the record, step3 of wf 11634.0 also runs a GlobalRecHitsAnalyzer which consumed both hltHbhereco and hltHbherecoLegacy (on CPU), see #40155 (comment).

Thanks @missirol. I was relying on the output of Tracer service with dumpPathsAndConsumes=True, and apparently that doesn't include consumesMany.

makortel · 2022-11-28T16:10:49Z

@cmsbuild, please test

#40153 is now merged (and thanks @missirol for the update!)

cmsbuild · 2022-11-28T22:23:06Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-420682/29307/summary.html
COMMIT: 3759817
CMSSW: CMSSW_13_0_X_2022-11-28-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40136/29307/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: 9 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3417311
DQMHistoTests: Total failures: 9
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3417280
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

makortel · 2022-11-29T02:13:12Z

+1

cmsbuild · 2022-11-29T02:13:38Z

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)

perrotta · 2022-11-29T06:11:08Z

+1

makortel added 2 commits November 23, 2022 03:04

Consolidate some testSwitchProducerTask tests

d1056c4

To reduce copy-paste

Make duplicate branch check work properly with SwitchProducer that ha…

3759817

…s EDAlias case Earlier check worked only if the non-SwitchProducer branch was in the map earlier.

cmsbuild added this to the CMSSW_12_6_X milestone Nov 23, 2022

cmsbuild added code-checks-pending core-pending orp-pending pending-signatures tests-pending labels Nov 23, 2022

This was referenced Nov 23, 2022

Segmentation violation in all DQM online clients during HI run #40110

Closed

Make duplicate branch check work properly with SwitchProducer that has EDAlias case cms-sw/framework-team#460

Closed

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

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

cmsbuild added tests-rejected and removed tests-started labels Nov 23, 2022

cmsbuild modified the milestones: CMSSW_12_6_X, CMSSW_13_0_X Nov 24, 2022

This was referenced Nov 25, 2022

HLT menu development for 12_5_X (2/N) [13_0_X] #40153

Merged

HLT menu development for 12_5_X (2/N) [12_6_X] #40154

Merged

HLT menu development for 12_5_X (2/N) #40155

Merged

cmsbuild added the tests-started label Nov 25, 2022

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

cmsbuild added tests-started and removed requires-external tests-approved labels Nov 28, 2022

makortel mentioned this pull request Nov 28, 2022

Migrate away from consumesMany() #40167

Closed

9 tasks

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

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

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

cmsbuild merged commit 830c070 into cms-sw:master Nov 29, 2022

makortel deleted the fixSwitchEDAliasCheck branch November 29, 2022 14:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make duplicate branch check work properly with SwitchProducer that has EDAlias case #40136

Make duplicate branch check work properly with SwitchProducer that has EDAlias case #40136

makortel commented Nov 23, 2022

cmsbuild commented Nov 23, 2022

cmsbuild commented Nov 23, 2022

makortel commented Nov 23, 2022

cmsbuild commented Nov 23, 2022

missirol commented Nov 23, 2022

Martin-Grunewald commented Nov 23, 2022 •

edited

missirol commented Nov 23, 2022

makortel commented Nov 23, 2022

missirol commented Nov 23, 2022 •

edited

cmsbuild commented Nov 26, 2022

missirol commented Nov 26, 2022

makortel commented Nov 28, 2022

makortel commented Nov 28, 2022

cmsbuild commented Nov 28, 2022

makortel commented Nov 29, 2022

cmsbuild commented Nov 29, 2022

perrotta commented Nov 29, 2022

Make duplicate branch check work properly with SwitchProducer that has EDAlias case #40136

Make duplicate branch check work properly with SwitchProducer that has EDAlias case #40136

Conversation

makortel commented Nov 23, 2022

PR description:

PR validation:

cmsbuild commented Nov 23, 2022

cmsbuild commented Nov 23, 2022

makortel commented Nov 23, 2022

cmsbuild commented Nov 23, 2022

RelVals

RelVals-INPUT

AddOn Tests

missirol commented Nov 23, 2022

Martin-Grunewald commented Nov 23, 2022 • edited

missirol commented Nov 23, 2022

makortel commented Nov 23, 2022

missirol commented Nov 23, 2022 • edited

cmsbuild commented Nov 26, 2022

Comparison Summary

missirol commented Nov 26, 2022

makortel commented Nov 28, 2022

makortel commented Nov 28, 2022

cmsbuild commented Nov 28, 2022

Comparison Summary

makortel commented Nov 29, 2022

cmsbuild commented Nov 29, 2022

perrotta commented Nov 29, 2022

Martin-Grunewald commented Nov 23, 2022 •

edited

missirol commented Nov 23, 2022 •

edited