Simplify setting EDPutTokenT via call to produces #34926

Dr15Jones · 2021-08-17T21:33:44Z

PR description:

Call to produces does not require specifying the data product type as it will be deduced via EDPutTokenT.
Also did some minor code modernization.

PR validation:

Code compiles.

cmsbuild · 2021-08-17T21:38:45Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34926/24734

This PR adds an extra 52KB to repository

cmsbuild · 2021-08-17T21:39:05Z

A new Pull Request was created by @Dr15Jones (Chris Jones) for master.

It involves the following packages:

FWCore/Framework (core)
FWCore/Modules (core)
FWCore/Utilities (core)

@makortel, @smuzaffar, @cmsbuild, @Dr15Jones can you please review it and eventually sign? Thanks.
@makortel, @felicepantaleo, @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 · 2021-08-17T22:28:52Z

@cmsbuild, please test

cmsbuild · 2021-08-18T00:49:29Z

-1

Failed Tests: Build HeaderConsistency ClangBuild
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8ff0b1/17847/summary.html
COMMIT: 07f14de
CMSSW: CMSSW_12_1_X_2021-08-17-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34926/17847/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:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8ff0b1/17847/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8ff0b1/17847/git-merge-result

Build

I found compilation error when building:

                 from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-08-17-1100/src/FWCore/Framework/interface/Event.h:35,
                 from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-08-17-1100/src/DQMServices/Core/interface/DQMEDAnalyzer.h:21,
                 from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-08-17-1100/src/CalibPPS/AlignmentGlobal/plugins/PPSAlignmentWorker.cc:13:
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-08-17-1100/src/FWCore/Utilities/interface/EDPutToken.h: In instantiation of 'constexpr edm::EDPutTokenT& edm::EDPutTokenT::operator=(ADAPTER&&) [with ADAPTER = edm::EDPutTokenT&; T = DQMToken]':
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-08-17-1100/src/DQMServices/Core/interface/DQMEDAnalyzer.h:72:45:   required from here
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-08-17-1100/src/FWCore/Utilities/interface/EDPutToken.h:88:45: error: 'class edm::EDPutTokenT' has no member named 'produces'
   88 |       EDPutTokenT temp(iAdapter.template produces());
      |                           ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
gmake: *** [tmp/slc7_amd64_gcc900/src/CalibPPS/AlignmentGlobal/plugins/CalibPPSAlignmentPlugins/PPSAlignmentWorker.cc.o] Error 1
>> Building  edm plugin tmp/slc7_amd64_gcc900/src/CalibPPS/AlignmentGlobal/plugins/CalibPPSAlignmentPlugins/libCalibPPSAlignmentPlugins.so
c++: error: tmp/slc7_amd64_gcc900/src/CalibPPS/AlignmentGlobal/plugins/CalibPPSAlignmentPlugins/PPSAlignmentWorker.cc.o: No such file or directory

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'

>> Compiling  /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-08-17-1100/src/FWCore/Framework/test/stream_filter_t.cppunit.cc
>> Compiling  /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-08-17-1100/src/FWCore/Framework/test/limited_producer_t.cppunit.cc
>> Compiling  /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-08-17-1100/src/FWCore/Framework/test/limited_filter_t.cppunit.cc
>> Compiling  /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-08-17-1100/src/FWCore/Framework/test/limited_outputmodule_t.cppunit.cc
>> Compiling  /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-08-17-1100/src/FWCore/Framework/test/checkForModuleDependencyCorrectness_t.cppunit.cc
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-08-17-1100/src/FWCore/Framework/test/maker2_t.cppunit.cc:30:41: error: no matching member function for call to 'produces'
TestMod::TestMod(ParameterSet const&) { produces(); }
                                        ^~~~~~~~~~~~~
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-08-17-1100/src/FWCore/Framework/interface/ProducerBase.h:112:24: note: candidate template ignored: invalid explicitly-specified argument for template parameter 'Tr'
    [[nodiscard]] auto produces() noexcept {
                       ^

Call to produces does not require specifying the data product type as it will be deduced via EDPutTokenT.

cmsbuild · 2021-08-18T13:11:52Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34926/24744

This PR adds an extra 52KB to repository

cmsbuild · 2021-08-18T13:12:15Z

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

Dr15Jones · 2021-08-18T13:13:04Z

please test

makortel · 2021-08-18T23:24:32Z

FWCore/Framework/interface/ProducesCollector.h

+    }
+
+  private:
+    //only ConsumesCollector is allowed to make an instance of this class


Should this be

Suggested change

//only ConsumesCollector is allowed to make an instance of this class

//only ProducesCollector is allowed to make an instance of this class

?

makortel · 2021-08-18T23:25:12Z

FWCore/Framework/interface/ProductRegistryHelper.h

+    }
+
+  private:
+    //only ConsumesCollector is allowed to make an instance of this class


Suggested change

//only ConsumesCollector is allowed to make an instance of this class

//only ProducesCollector is allowed to make an instance of this class

cmsbuild · 2021-08-20T16:00:23Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34926/24800

This PR adds an extra 56KB to repository

cmsbuild · 2021-08-20T16:00:49Z

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

Dr15Jones · 2021-08-20T16:05:45Z

please test

cmsbuild · 2021-08-20T21:44:18Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8ff0b1/17927/summary.html
COMMIT: 87e056e
CMSSW: CMSSW_12_1_X_2021-08-20-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34926/17927/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: 39
DQMHistoTests: Total histograms compared: 3000352
DQMHistoTests: Total failures: 5
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3000324
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 38 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 165 log files, 37 edm output root files, 39 DQM output files
TriggerResults: no differences found

makortel · 2021-08-20T23:55:00Z

+1

cmsbuild · 2021-08-20T23:55:19Z

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

qliphy · 2021-08-21T01:27:44Z

+1

cmsbuild added this to the CMSSW_12_1_X milestone Aug 17, 2021

cmsbuild added code-checks-pending core-pending orp-pending pending-signatures tests-pending labels Aug 17, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 17, 2021

makortel mentioned this pull request Aug 17, 2021

add type deduction for produces calls using put tokens #34906

Closed

cmsbuild added tests-started and removed tests-pending labels Aug 17, 2021

makortel mentioned this pull request Aug 17, 2021

Add type deduction for produces calls using put tokens cms-sw/framework-team#222

Closed

cmsbuild added tests-rejected and removed tests-started labels Aug 18, 2021

Simplify setting EDPutTokenT via call to produces

4011a22

Call to produces does not require specifying the data product type as it will be deduced via EDPutTokenT.

Dr15Jones force-pushed the simpler_produces branch from 07f14de to 4011a22 Compare August 18, 2021 13:06

cmsbuild added code-checks-pending tests-pending and removed tests-rejected code-checks-approved labels Aug 18, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 18, 2021

cmsbuild removed the tests-pending label Aug 18, 2021

cmsbuild added the tests-approved label Aug 18, 2021

makortel reviewed Aug 18, 2021

View reviewed changes

Corrected comments

87e056e

cmsbuild added code-checks-pending tests-pending and removed tests-approved code-checks-approved labels Aug 20, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 20, 2021

cmsbuild added tests-started and removed tests-pending labels Aug 20, 2021

cmsbuild added tests-approved and removed tests-started labels Aug 20, 2021

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Aug 20, 2021

cmsbuild added orp-approved and removed orp-pending labels Aug 21, 2021

cmsbuild merged commit d47b94b into cms-sw:master Aug 21, 2021

cmsbuild mentioned this pull request Aug 21, 2021

Run3-hcx310 Put back HcalDigiProducer.cc in SimCalorimetry/HcalSimProducers/src #34972

Merged

Dr15Jones deleted the simpler_produces branch August 24, 2021 18:27

cmsbuild mentioned this pull request Aug 27, 2021

DataFormats/FEDRawData UBSAN warning fixed #35039

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify setting EDPutTokenT via call to produces #34926

Simplify setting EDPutTokenT via call to produces #34926

Dr15Jones commented Aug 17, 2021

cmsbuild commented Aug 17, 2021

cmsbuild commented Aug 17, 2021

makortel commented Aug 17, 2021

cmsbuild commented Aug 18, 2021

cmsbuild commented Aug 18, 2021

cmsbuild commented Aug 18, 2021

Dr15Jones commented Aug 18, 2021

makortel Aug 18, 2021

makortel Aug 18, 2021

cmsbuild commented Aug 20, 2021

cmsbuild commented Aug 20, 2021

Dr15Jones commented Aug 20, 2021

cmsbuild commented Aug 20, 2021

makortel commented Aug 20, 2021

cmsbuild commented Aug 20, 2021

qliphy commented Aug 21, 2021

	//only ConsumesCollector is allowed to make an instance of this class
	//only ProducesCollector is allowed to make an instance of this class

Simplify setting EDPutTokenT via call to produces #34926

Simplify setting EDPutTokenT via call to produces #34926

Conversation

Dr15Jones commented Aug 17, 2021

PR description:

PR validation:

cmsbuild commented Aug 17, 2021

cmsbuild commented Aug 17, 2021

makortel commented Aug 17, 2021

cmsbuild commented Aug 18, 2021

Build

Clang Build

cmsbuild commented Aug 18, 2021

cmsbuild commented Aug 18, 2021

Dr15Jones commented Aug 18, 2021

makortel Aug 18, 2021

Choose a reason for hiding this comment

makortel Aug 18, 2021

Choose a reason for hiding this comment

cmsbuild commented Aug 20, 2021

cmsbuild commented Aug 20, 2021

Dr15Jones commented Aug 20, 2021

cmsbuild commented Aug 20, 2021

Comparison Summary

makortel commented Aug 20, 2021

cmsbuild commented Aug 20, 2021

qliphy commented Aug 21, 2021