Refactor ProductResolvers #33230

Dr15Jones · 2021-03-19T20:31:10Z

PR description:

Removed put interfaces from base class and use ProductPutterBase abstract class instead
Separated InputProductResolver into DelayedReader* and PutOnRead* variants
BranchDescription::onDemand is also true for case of DelayedReader being used by source
putOrMerge on ProductResolvers was not being used so was removed.

PR validation:

code compiles
framework related unit tests all pass.

cmsbuild · 2021-03-19T20:38:19Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33230/21673

This PR adds an extra 96KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-checks:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33230/21673/code-checks.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33230/21673/code-checks.patch | patch -p1
You can also run scram build code-checks to apply code checks directly

-Removed put interfaces from base class and use ProductPutterBase abstract class instead -Separated InputProductResolver into DelayedReader* and PutOnRead* variants -BranchDescription::onDemand is also true for case of DelayedReader being used by source -putOrMerge on ProductResolvers was not being used so was removed.

cmsbuild · 2021-03-19T20:57:40Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33230/21675

This PR adds an extra 96KB to repository

cmsbuild · 2021-03-19T20:57:58Z

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

It involves the following packages:

DataFormats/Provenance
FWCore/Framework
FWCore/TFWLiteSelector
FWCore/TestProcessor
IOPool/Input

@makortel, @smuzaffar, @cmsbuild, @Dr15Jones can you please review it and eventually sign? Thanks.
@makortel, @wddgit, @rovere this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

Dr15Jones · 2021-03-19T20:58:27Z

please test

cmsbuild · 2021-03-20T05:22:05Z

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7753ae/13637/summary.html
COMMIT: 14f18b0
CMSSW: CMSSW_11_3_X_2021-03-19-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33230/13637/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:

drop type spec and move sequence to task in CommonTools #33201 @jeongeun: drop type spec and move sequence to task in CommonTools
Move FFTJet to JetMETCorrections/FFTJetObjects #33179 @davidlange6: Move FFTJet to JetMETCorrections/FFTJetObjects

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

Unit Tests

I found errors in the following unit tests:

---> test TestGeneratorInterfaceLHEInterfaceFileReading had ERRORS
---> test testGeneratorInterfaceLHEInterface_TP had ERRORS

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2639935
DQMHistoTests: Total failures: 7
DQMHistoTests: Total nulls: 6
DQMHistoTests: Total successes: 2639900
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -5.491 KiB( 36 files compared)
DQMHistoSizes: changed ( 1000.0,... ): -0.145 KiB MessageLogger/Warnings
DQMHistoSizes: changed ( 1000.0,... ): -0.145 KiB MessageLogger/Errors
Checked 155 log files, 37 edm output root files, 37 DQM output files
TriggerResults: found differences in 2 / 32 workflows

-Added unit test for this case.

cmsbuild · 2021-03-24T14:59:15Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33230/21739

This PR adds an extra 112KB to repository

cmsbuild · 2021-03-25T18:24:07Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33230/21773

This PR adds an extra 116KB to repository

cmsbuild · 2021-03-25T18:24:29Z

Pull request #33230 was updated. @SiewYan, @smuzaffar, @Dr15Jones, @mkirsano, @makortel, @cmsbuild, @GurpreetSinghChahal, @agrohsje, @alberto-sanchez can you please check and sign again.

Dr15Jones · 2021-03-25T18:59:35Z

please test with #33274

cmsbuild · 2021-03-26T00:05:06Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7753ae/13777/summary.html
COMMIT: 72b9481
CMSSW: CMSSW_11_3_X_2021-03-25-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33230/13777/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: 37
DQMHistoTests: Total histograms compared: 2639935
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2639912
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
Checked 155 log files, 37 edm output root files, 37 DQM output files
TriggerResults: no differences found

cmsbuild · 2021-03-26T00:05:07Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7753ae/13777/summary.html
COMMIT: 72b9481
CMSSW: CMSSW_11_3_X_2021-03-25-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33230/13777/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: 37
DQMHistoTests: Total histograms compared: 2639935
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2639912
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
Checked 155 log files, 37 edm output root files, 37 DQM output files
TriggerResults: no differences found

agrohsje · 2021-03-26T09:13:48Z

no objection to the gen part.
+generators

makortel · 2021-03-26T13:26:55Z

+1

qliphy · 2021-03-28T07:15:51Z

no objection to the gen part.
+generators

Not sure why the signature doesn't work. Anyway, thanks!

qliphy · 2021-03-28T07:16:00Z

+1

qliphy · 2021-03-28T07:16:30Z

merge

agrohsje · 2021-03-29T09:55:36Z

+generators

cmsbuild · 2021-03-29T09:55:58Z

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 be automatically merged.

cmsbuild added this to the CMSSW_11_3_X milestone Mar 19, 2021

cmsbuild added code-checks-pending core-pending orp-pending pending-signatures tests-pending labels Mar 19, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Mar 19, 2021

Dr15Jones force-pushed the refactorProductRetrievers branch from 8efc914 to 14f18b0 Compare March 19, 2021 20:52

cmsbuild added code-checks-pending and removed code-checks-rejected labels Mar 19, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 19, 2021

cmsbuild added tests-started and removed tests-pending labels Mar 19, 2021

cmsbuild added tests-rejected and removed tests-started labels Mar 20, 2021

Dr15Jones added 2 commits March 24, 2021 09:49

Add explicit constructor to set value

243c880

Add back putOrMerge interface to RunPrincipal

dac6294

-Added unit test for this case.

cmsbuild added code-checks-pending tests-pending and removed code-checks-approved tests-rejected labels Mar 24, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 25, 2021

cmsbuild added requires-external tests-started and removed tests-pending labels Mar 25, 2021

cmsbuild added tests-approved and removed tests-started labels Mar 26, 2021

cmsbuild added core-approved and removed core-pending labels Mar 26, 2021

cmsbuild added orp-approved and removed orp-pending labels Mar 28, 2021

cmsbuild merged commit 65b3d04 into cms-sw:master Mar 28, 2021

cmsbuild added fully-signed generators-approved and removed generators-pending pending-signatures labels Mar 29, 2021

Dr15Jones deleted the refactorProductRetrievers branch April 2, 2021 15:34

smuzaffar mentioned this pull request Sep 16, 2021

Make merge interface of Principal only called by Sources #12946

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor ProductResolvers #33230

Refactor ProductResolvers #33230

Dr15Jones commented Mar 19, 2021

cmsbuild commented Mar 19, 2021

cmsbuild commented Mar 19, 2021

cmsbuild commented Mar 19, 2021

Dr15Jones commented Mar 19, 2021

cmsbuild commented Mar 20, 2021

cmsbuild commented Mar 24, 2021

cmsbuild commented Mar 25, 2021

cmsbuild commented Mar 25, 2021

Dr15Jones commented Mar 25, 2021

cmsbuild commented Mar 26, 2021

cmsbuild commented Mar 26, 2021

agrohsje commented Mar 26, 2021

makortel commented Mar 26, 2021

qliphy commented Mar 28, 2021 •

edited

qliphy commented Mar 28, 2021

qliphy commented Mar 28, 2021

agrohsje commented Mar 29, 2021

cmsbuild commented Mar 29, 2021

Refactor ProductResolvers #33230

Refactor ProductResolvers #33230

Conversation

Dr15Jones commented Mar 19, 2021

PR description:

PR validation:

cmsbuild commented Mar 19, 2021

cmsbuild commented Mar 19, 2021

cmsbuild commented Mar 19, 2021

Dr15Jones commented Mar 19, 2021

cmsbuild commented Mar 20, 2021

Unit Tests

Comparison Summary

cmsbuild commented Mar 24, 2021

cmsbuild commented Mar 25, 2021

cmsbuild commented Mar 25, 2021

Dr15Jones commented Mar 25, 2021

cmsbuild commented Mar 26, 2021

Comparison Summary

cmsbuild commented Mar 26, 2021

Comparison Summary

agrohsje commented Mar 26, 2021

makortel commented Mar 26, 2021

qliphy commented Mar 28, 2021 • edited

qliphy commented Mar 28, 2021

qliphy commented Mar 28, 2021

agrohsje commented Mar 29, 2021

cmsbuild commented Mar 29, 2021

qliphy commented Mar 28, 2021 •

edited