[RFC] Add Transformer ability to modules
PR description:
Added ability to register tranform methods to modules. This allows a module to have a method which takes a data product already put into the Event by the same module and then create a new data product from it which is also put into the Event.
The plan is to use this ability to implement an automatic conversion from GPU to CPU data products.
PR validation:
Code compiles and new unit test passes.
+code-checks
Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38454/30666
- This PR adds an extra 200KB to repository
A new Pull Request was created by @Dr15Jones (Chris Jones) for master.
It involves the following packages:
- DataFormats/Provenance (core)
- FWCore/Framework (core)
- FWCore/Integration (core)
@cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please review it and eventually sign? Thanks. @makortel, @wddgit, @rovere 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
please test
-1
Failed Tests: Build ClangBuild
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-49175f/25673/summary.html
COMMIT: 73463055968d7500803837dd1b80a7cd0ccca89d
CMSSW: CMSSW_12_5_X_2022-06-21-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/38454/25673/install.sh to create a dev area with all the needed externals and cmssw changes.
The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
- @fabiocos cms-sw/cmssw#38419
- @missirol cms-sw/cmssw#38418
- @cms-sw cms-sw/cmssw#38451
- @jeongeun cms-sw/cmssw#38374
- @trackreco cms-sw/cmssw#38437
You can see more details here: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-49175f/25673/git-recent-commits.json https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-49175f/25673/git-merge-result
Build
I found compilation error when building:
>> Leaving Package AnalysisAlgos/SiStripClusterInfoProducer >> Package AnalysisAlgos/SiStripClusterInfoProducer built Entering library rule at src/AnalysisAlgos/SiStripClusterInfoProducer/plugins >> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-06-21-1100/src/AnalysisAlgos/SiStripClusterInfoProducer/plugins/SiStripProcessedRawDigiProducer.cc /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-06-21-1100/src/AnalysisAlgos/SiStripClusterInfoProducer/plugins/SiStripProcessedRawDigiProducer.cc: In member function 'void SiStripProcessedRawDigiProducer::pr_process(const edm::DetSetVector&, edm::DetSetVector &, const SiStripGain&)': /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-06-21-1100/src/AnalysisAlgos/SiStripClusterInfoProducer/plugins/SiStripProcessedRawDigiProducer.cc:126:5: error: 'virtual void edm::stream::EDProducerBase::transform(std::size_t, edm::EventForTransformer&) const' is private within this context 126 | transform( | ^~~~~~~~~ In file included from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-06-21-1100/src/FWCore/Framework/interface/stream/implementors.h:31, from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-06-21-1100/src/FWCore/Framework/interface/stream/AbilityToImplementor.h:25, from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-06-21-1100/src/FWCore/Framework/interface/stream/EDProducer.h:22,
Clang Build
I found compilation error while trying to compile with clang. Command used:
USER_CUDA_FLAGS='--expt-relaxed-constexpr' USER_CXXFLAGS='-Wno-register -fsyntax-only' scram build -k -j 32 COMPILER='llvm compile'
>> Entering Package RecoMuon/MuonSeedGenerator >> Entering Package RecoMuon/StandAloneMuonProducer >> Entering Package SimG4Core/Application >> Entering Package SimGeneral/MixingModule >> Compile sequence completed for CMSSW CMSSW_12_5_X_2022-06-21-1100 gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 1 + eval scram build outputlog '&&' '(python3' /data/cmsbld/jenkins/workspace/ib-run-pr-tests/cms-bot/buildLogAnalyzer.py --ignoreWarning=Wdeprecated-declarations --logDir /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-06-21-1100/tmp/el8_amd64_gcc10/cache/log/src '||' 'true)' ++ scram build outputlog >> Entering Package DataFormats/Provenance Entering library rule at DataFormats/Provenance >> Compiling /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-06-21-1100/src/DataFormats/Provenance/src/BranchChildren.cc
please test
+code-checks
Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38454/30682
- This PR adds an extra 20KB to repository
Pull request #38454 was updated. @makortel, @smuzaffar, @Dr15Jones can you please check and sign again.
-1
Failed Tests: Build
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-49175f/25701/summary.html
COMMIT: 998a6287246d7c7e28c4e748024961d48931ef7c
CMSSW: CMSSW_12_5_X_2022-06-22-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/38454/25701/install.sh to create a dev area with all the needed externals and cmssw changes.
Build
I found compilation error when building:
>> Leaving Package AnalysisAlgos/SiStripClusterInfoProducer >> Package AnalysisAlgos/SiStripClusterInfoProducer built Entering library rule at src/AnalysisAlgos/SiStripClusterInfoProducer/plugins >> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-06-22-1100/src/AnalysisAlgos/SiStripClusterInfoProducer/plugins/SiStripProcessedRawDigiProducer.cc /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-06-22-1100/src/AnalysisAlgos/SiStripClusterInfoProducer/plugins/SiStripProcessedRawDigiProducer.cc: In member function 'void SiStripProcessedRawDigiProducer::pr_process(const edm::DetSetVector&, edm::DetSetVector &, const SiStripGain&)': /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-06-22-1100/src/AnalysisAlgos/SiStripClusterInfoProducer/plugins/SiStripProcessedRawDigiProducer.cc:126:5: error: 'virtual void edm::stream::EDProducerBase::transform(std::size_t, edm::EventForTransformer&) const' is private within this context 126 | transform( | ^~~~~~~~~ In file included from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-06-22-1100/src/FWCore/Framework/interface/stream/implementors.h:31, from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-06-22-1100/src/FWCore/Framework/interface/stream/AbilityToImplementor.h:25, from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-06-22-1100/src/FWCore/Framework/interface/stream/EDProducer.h:22,
+code-checks
Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38454/30690
- This PR adds an extra 48KB to repository
Pull request #38454 was updated. @cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please check and sign again.
please test
+1
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-49175f/25708/summary.html
COMMIT: ba50de5fbaf2033bd54dcbe8277d89d607a168ed
CMSSW: CMSSW_12_5_X_2022-06-22-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/38454/25708/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: 0 differences found in the comparisons
- DQMHistoTests: Total files compared: 50
- DQMHistoTests: Total histograms compared: 3659099
- DQMHistoTests: Total failures: 2
- DQMHistoTests: Total nulls: 0
- DQMHistoTests: Total successes: 3659075
- DQMHistoTests: Total skipped: 22
- DQMHistoTests: Total Missing objects: 0
- DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
- Checked 208 log files, 45 edm output root files, 50 DQM output files
- TriggerResults: no differences found
One additional thought (that I have been forgetting): the first use case for this feature would be device-to-host transfers, that cab be done asynchronously. With the generalized SoA approach I'd expect vast majority of cases to be along
- construct SoA in (pinned) host memory
- launch the asynchronous copy
- return the host-side SoA
i.e. the data product does not need to be changed after the asynchronous copy has finished. But I can't exclude a corner case where such functionality would be useful or needed (although that part can not be fully automated).
I can quickly think of a few options for that (there are probably more):
- keep current
registerTransform()function, but addedm::WaitingTaskWithArenaHolderparameter. Workers depending on the transformed product would become eligible to run after theWaitingTaskWithArenaHolder::doneWaiting()has been called (i.e. similar toacquire()phase ofExternalWork`) - same as 1 but rename the function to
registerTransformAsync()or something - keep the current
registerTransform()functionality for synchronous transformation, and add separateregisterTransformAsync()withedm::WaitingTaskArenaHolderparameter etc - same as 3, and add a third function that adds possibility for running code after the asynchronous work has finished (similar to
produce()phase of `ExternalWork)
I would not necessarily aim for the most general solution now (as long as there is a path to extend towards that later).
please test
+code-checks
Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38454/31211
-
This PR adds an extra 156KB to repository
-
There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/Provenance/interface/BranchDescription.h modified in PR(s): #38806
- File FWCore/Framework/test/global_filter_t.cppunit.cc modified in PR(s): #38801
- File FWCore/Framework/test/global_producer_t.cppunit.cc modified in PR(s): #38801
- File FWCore/Framework/test/limited_filter_t.cppunit.cc modified in PR(s): #38801
- File FWCore/Framework/test/limited_producer_t.cppunit.cc modified in PR(s): #38801
Pull request #38454 was updated. @makortel, @smuzaffar, @Dr15Jones can you please check and sign again.
-1
Failed Tests: ClangBuild
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-49175f/26397/summary.html
COMMIT: 18ea41a80a77bb5f76bf0effa64ae2512c2da95f
CMSSW: CMSSW_12_5_X_2022-07-22-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/38454/26397/install.sh to create a dev area with all the needed externals and cmssw changes.
Clang Build
I found compilation error while trying to compile with clang. Command used:
USER_CUDA_FLAGS='--expt-relaxed-constexpr' USER_CXXFLAGS='-Wno-register -fsyntax-only' scram build -k -j 32 COMPILER='llvm compile'
>> Creating project symlinks >> Entering Package DataFormats/Provenance >> Entering Package FWCore/Framework >> Entering Package FWCore/Integration >> Compile sequence completed for CMSSW CMSSW_12_5_X_2022-07-22-1100 gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 1 + eval scram build outputlog '&&' '(python3' /data/cmsbld/jenkins/workspace/ib-run-pr-tests/cms-bot/buildLogAnalyzer.py --ignoreWarning=Wdeprecated-declarations --logDir /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-07-22-1100/tmp/el8_amd64_gcc10/cache/log/src '||' 'true)' ++ scram build outputlog >> Entering Package DataFormats/Provenance Entering library rule at DataFormats/Provenance >> Compiling /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-07-22-1100/src/DataFormats/Provenance/src/BranchChildren.cc
please test
+code-checks
Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38454/31215
-
This PR adds an extra 156KB to repository
-
There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/Provenance/interface/BranchDescription.h modified in PR(s): #38806
- File FWCore/Framework/test/global_filter_t.cppunit.cc modified in PR(s): #38801
- File FWCore/Framework/test/global_producer_t.cppunit.cc modified in PR(s): #38801
- File FWCore/Framework/test/limited_filter_t.cppunit.cc modified in PR(s): #38801
- File FWCore/Framework/test/limited_producer_t.cppunit.cc modified in PR(s): #38801
Pull request #38454 was updated. @makortel, @smuzaffar, @Dr15Jones can you please check and sign again.
An asynchronous transform ability with two parts could look like
transformAsync( m_fooPutToken,
[](Foo const& iFoo, auto iTask) { return setupWorkAndReturnResultsPtr(iFoo, iTask);},
[](auto iHolder) { return *iHolder; });
Where the item returned from the first lambda is moved and then moved back to the second lambda once the task is complete.
An asynchronous transform ability with two parts could look like
transformAsync( m_fooPutToken, [](Foo const& iFoo, auto iTask) { return setupWorkAndReturnResultsPtr(iFoo, iTask);}, [](auto iHolder) { return *iHolder; });Where the item returned from the first lambda is moved and then moved back to the second lambda once the task is complete.
Is the type of of iTask edm::WaitingTaskwithArenaHolder and the type of iHolder whatever the setupWorkAndReturnResultsPtr() returns?
Is the type of of iTask edm::WaitingTaskwithArenaHolder
Yes
and the type of iHolder whatever the setupWorkAndReturnResultsPtr() returns?
Yes
The interface looks reasonable to me. I only wonder if the task structure can be simple-enough for just moving the intermediate data around (in contrast to ExternalWork #24186).
-1
Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-49175f/26400/summary.html
COMMIT: 771df138c64c8e36d749ed1314ce2fb6fcbea563
CMSSW: CMSSW_12_5_X_2022-07-22-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/38454/26400/install.sh to create a dev area with all the needed externals and cmssw changes.
RelVals-INPUT
The relvals timed out after 4 hours.
Comparison Summary
Summary:
- No significant changes to the logs found
- Reco comparison results: 2 differences found in the comparisons
- DQMHistoTests: Total files compared: 51
- DQMHistoTests: Total histograms compared: 3706484
- DQMHistoTests: Total failures: 8
- DQMHistoTests: Total nulls: 0
- DQMHistoTests: Total successes: 3706454
- DQMHistoTests: Total skipped: 22
- DQMHistoTests: Total Missing objects: 0
- DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
- Checked 210 log files, 47 edm output root files, 51 DQM output files
- TriggerResults: no differences found
please test
+1
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-49175f/26445/summary.html
COMMIT: 771df138c64c8e36d749ed1314ce2fb6fcbea563
CMSSW: CMSSW_12_5_X_2022-07-25-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38454/26445/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: 51
- DQMHistoTests: Total histograms compared: 3667670
- DQMHistoTests: Total failures: 2
- DQMHistoTests: Total nulls: 0
- DQMHistoTests: Total successes: 3667646
- DQMHistoTests: Total skipped: 22
- DQMHistoTests: Total Missing objects: 0
- DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
- Checked 210 log files, 47 edm output root files, 51 DQM output files
- TriggerResults: no differences found
Hi, @Dr15Jones , how are we doing on this one? Looks like it missed the 12_5 milestone but does it need a backport there now?
Hi, @Dr15Jones , how are we doing on this one? Looks like it missed the 12_5 milestone but does it need a backport there now?
@makortel what do you want to do?
Hi, @Dr15Jones , how are we doing on this one? Looks like it missed the 12_5 milestone but does it need a backport there now?
@makortel what do you want to do?
I think 12_6_X is sufficient.