Make SwitchProducer work with ConditionalTask #37563

makortel · 2022-04-14T00:21:03Z

PR description:

Further testing of ConditionalTask (#37305) for HLT revealed that it and SwitchProducer mechanism didn't work together at all (as discussed in #36938 (comment) onwards). This PR makes the SwitchProducer to work with ConditionalTask with the following changes

SwitchProducer case module objects get now their module labels set (previously they were not).
- This change lead to Move pixelTracksCPU module definition inside SwitchProducerCUDA #37562. I checked all other uses of SwitchProducerCUDA and the case modules were always either defined inline or cloned from other module objects
In python, if SwitchProducer is on a ConditionalTask or in a Path, the case modules get treated as if they were in an anonymous ConditionalTask associated with the ConditionalTask/Path
SwitchProducer with an EDAlias as a chosen case required adding those EDAliases into the ProductRegistry so that the callWhenNewProductsRegistered() callbacks in SwitchProducer get called, that lead to data product consumption being properly registered, that is needed for the module dependence checks in StreamSchedule::tryToPlaceConditionalModules()
- This step required moving processEDAliases() function out from Schedule.cc
The tests I added for SwitchProducer with EDAlias also revealed that the EDAlias treatment was generally broken for ConditionalTask.

PR validation:

Framework unit tests run. The HLT test in #36938 (comment) runs as well.

cmsbuild · 2022-04-14T00:27:38Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37563/29295

This PR adds an extra 120KB to repository

cmsbuild · 2022-04-14T00:28:05Z

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

It involves the following packages:

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

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

makortel · 2022-04-14T00:30:36Z

enable gpu

makortel · 2022-04-14T00:32:47Z

FWCore/Framework/src/processEDAliases.cc

+          if (not aliasModulesToProcess.empty() and
+              aliasModulesToProcess.find(moduleLabel) == aliasModulesToProcess.end()) {
+            continue;
+          }


I added the aliasModulesToProcess argument and these lines when extracting the processEDAliases() function into a separate file.

makortel · 2022-04-14T00:35:05Z

FWCore/ParameterSet/python/Config.py

@@ -1298,7 +1298,7 @@ def _insertPaths(self, processPSet, nodeVisitor):
              l.sort()
              decoratedList.extend(l)
              decoratedList.append("@")
-            iPath.insertInto(processPSet, triggername, decoratedList)
+            iPath.insertInto(processPSet, triggername, decoratedList[:])


decoratedList is copied because otherwise the same list object would be shared among the Paths, and the contents would be that of the last Path.

makortel · 2022-04-15T00:50:34Z

@cmsbuild, please test with #37562

cmsbuild · 2022-04-15T06:32:24Z

-1

Failed Tests: UnitTests RelVals
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-606e7a/23935/summary.html
COMMIT: a964d75
CMSSW: CMSSW_12_4_X_2022-04-14-2300/slc7_amd64_gcc10
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/37563/23935/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test testTauEmbeddingProducers had ERRORS

RelVals

101.0101.0_SingleElectronE120EHCAL+SingleElectronE120EHCAL/step1_SingleElectronE120EHCAL+SingleElectronE120EHCAL.log

GPU Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
Reco comparison had 3 failed jobs
DQMHistoTests: Total files compared: 4
DQMHistoTests: Total histograms compared: 19874
DQMHistoTests: Total failures: 954
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 18920
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 3 files compared)
Checked 12 log files, 9 edm output root files, 4 DQM output files
TriggerResults: no differences found

cmsbuild · 2022-04-15T13:29:17Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37563/29335

This PR adds an extra 56KB to repository

cmsbuild · 2022-04-20T01:03:50Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37563/29402

This PR adds an extra 120KB to repository

cmsbuild · 2022-04-20T01:04:17Z

Pull request #37563 was updated. @cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please check and sign again.

makortel · 2022-04-20T01:34:24Z

@cmsbuild, please test

Addressed Chris' comments.

cmsbuild · 2022-04-20T07:28:06Z

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-606e7a/24035/summary.html
COMMIT: ecdb11a
CMSSW: CMSSW_12_4_X_2022-04-19-2300/slc7_amd64_gcc10
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/37563/24035/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test TestDQMOnlineClient-l1tstage2_dqm_sourceclient had ERRORS

GPU Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
Reco comparison had 3 failed jobs
DQMHistoTests: Total files compared: 4
DQMHistoTests: Total histograms compared: 19874
DQMHistoTests: Total failures: 997
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 18877
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 3 files compared)
Checked 12 log files, 9 edm output root files, 4 DQM output files
TriggerResults: no differences found

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 12 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3589937
DQMHistoTests: Total failures: 18
DQMHistoTests: Total nulls: 10
DQMHistoTests: Total successes: 3589887
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 1.9979999999999998 KiB( 47 files compared)
DQMHistoSizes: changed ( 1000.0,... ): 0.031 KiB MessageLogger/Errors
DQMHistoSizes: changed ( 1000.0,... ): 0.031 KiB MessageLogger/Warnings
DQMHistoSizes: changed ( 11634.0,... ): 0.027 KiB MessageLogger/Errors
DQMHistoSizes: changed ( 11634.0,... ): 0.027 KiB MessageLogger/Warnings
DQMHistoSizes: changed ( 23234.0,... ): 0.023 KiB MessageLogger/Errors
DQMHistoSizes: changed ( 23234.0,... ): 0.023 KiB MessageLogger/Warnings
DQMHistoSizes: changed ( 4.22,... ): 0.012 KiB MessageLogger/Errors
DQMHistoSizes: changed ( 4.22,... ): 0.012 KiB MessageLogger/Warnings
Checked 200 log files, 45 edm output root files, 48 DQM output files
TriggerResults: no differences found

makortel · 2022-04-20T14:04:34Z

+1

The unit test fails already in the IBs

cmsbuild · 2022-04-20T14:04:52Z

This pull request is fully signed and it will be integrated in one of the next master IBs (but tests are reportedly failing). 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)

qliphy · 2022-04-20T15:53:07Z

merge

cmsbuild added this to the CMSSW_12_4_X milestone Apr 14, 2022

cmsbuild added code-checks-pending core-pending orp-pending pending-signatures tests-pending labels Apr 14, 2022

This was referenced Apr 14, 2022

Address modules in ConditionalTask that are consumed only by SwitchProducer in the Path cms-sw/framework-team#378

Closed

on running unscheduled HLT modules together with other "steps" #36938

Open

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 14, 2022

makortel commented Apr 14, 2022

View reviewed changes

makortel mentioned this pull request Apr 14, 2022

Move pixelTracksCPU module definition inside SwitchProducerCUDA #37562

Merged

cmsbuild added requires-external tests-started and removed tests-pending labels Apr 15, 2022

cmsbuild added tests-rejected and removed tests-started labels Apr 15, 2022

makortel force-pushed the conditionalTaskSwitchProducer branch from a964d75 to 20a4549 Compare April 15, 2022 13:22

cmsbuild added code-checks-pending tests-pending and removed tests-rejected requires-external code-checks-approved labels Apr 15, 2022

Fix EDAlias treatment for ConditionalTask

ecdb11a

makortel force-pushed the conditionalTaskSwitchProducer branch from 20a4549 to ecdb11a Compare April 20, 2022 00:56

cmsbuild added code-checks-pending tests-pending and removed tests-rejected code-checks-approved labels Apr 20, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 20, 2022

cmsbuild added tests-started and removed tests-pending labels Apr 20, 2022

cmsbuild added tests-rejected and removed tests-started labels Apr 20, 2022

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Apr 20, 2022

cmsbuild added orp-approved and removed orp-pending labels Apr 20, 2022

cmsbuild merged commit 9554e9c into cms-sw:master Apr 20, 2022

cmsbuild mentioned this pull request Apr 21, 2022

[CXXMODULE] Update to latest root master changes cms-sw/cmsdist#7800

Merged

fwyzard mentioned this pull request May 17, 2022

Backport ConditionalTask to CMSSW_12_3_X #37977

Closed

AdrianoDee mentioned this pull request May 24, 2022

Add PF developments for L1T phase-2 #38047

Closed

makortel deleted the conditionalTaskSwitchProducer branch October 4, 2023 19:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make SwitchProducer work with ConditionalTask #37563

Make SwitchProducer work with ConditionalTask #37563

makortel commented Apr 14, 2022

cmsbuild commented Apr 14, 2022

cmsbuild commented Apr 14, 2022

makortel commented Apr 14, 2022

makortel Apr 14, 2022

makortel Apr 14, 2022

makortel commented Apr 15, 2022

cmsbuild commented Apr 15, 2022

cmsbuild commented Apr 15, 2022

cmsbuild commented Apr 20, 2022

cmsbuild commented Apr 20, 2022

makortel commented Apr 20, 2022

cmsbuild commented Apr 20, 2022

makortel commented Apr 20, 2022

cmsbuild commented Apr 20, 2022

qliphy commented Apr 20, 2022

Make SwitchProducer work with ConditionalTask #37563

Make SwitchProducer work with ConditionalTask #37563

Conversation

makortel commented Apr 14, 2022

PR description:

PR validation:

cmsbuild commented Apr 14, 2022

cmsbuild commented Apr 14, 2022

makortel commented Apr 14, 2022

makortel Apr 14, 2022

Choose a reason for hiding this comment

makortel Apr 14, 2022

Choose a reason for hiding this comment

makortel commented Apr 15, 2022

cmsbuild commented Apr 15, 2022

Unit Tests

RelVals

GPU Comparison Summary

cmsbuild commented Apr 15, 2022

cmsbuild commented Apr 20, 2022

cmsbuild commented Apr 20, 2022

makortel commented Apr 20, 2022

cmsbuild commented Apr 20, 2022

Unit Tests

GPU Comparison Summary

Comparison Summary

makortel commented Apr 20, 2022

cmsbuild commented Apr 20, 2022

qliphy commented Apr 20, 2022