Moved construction of ProductResolvers to a separate function #47051

Dr15Jones · 2025-01-07T19:26:44Z

PR description:

Refactored how ProductResolvers are constructed. This should allow easier changes for the future.

PR validation:

Code compiles and all framework tests pass.

resolves cms-sw/framework-team#1145

cmsbuild · 2025-01-07T19:27:05Z

cms-bot internal usage

cmsbuild · 2025-01-07T19:30:44Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47051/43196

ERROR: Build errors found during clang-tidy run.

src/FWCore/Framework/src/ProductResolversFactory.cc:1:10: error: 'ProductResolversFactory.h' file not found [clang-diagnostic-error]
    1 | #include "ProductResolversFactory.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~
Suppressed 220 warnings (220 in non-user code).
--
gmake: *** [config/SCRAM/GMake/Makefile.coderules:129: code-checks] Error 2
gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 2

Dr15Jones · 2025-01-07T19:34:12Z

FWCore/Framework/src/Principal.cc

+        processHistoryID_(),
+        processHistoryIDBeforeConfig_(),
+        processConfiguration_(&pc),
+        productResolvers_(resolvers.begin(), resolvers.end()),


Note, the type of productResolvers_ and resolvers is slightly different (since productResolvers_ uses the edm:propagate_const on the elements) which is why I need to use the iterators.

cmsbuild · 2025-01-07T20:40:06Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47051/43197

There are other open Pull requests which might conflict with changes you have proposed:
- File FWCore/Framework/src/EventProcessor.cc modified in PR(s): Print ES module dependences from Tracer #46929
- File FWCore/Framework/src/SubProcess.cc modified in PR(s): Print ES module dependences from Tracer #46929

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

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

cmsbuild · 2025-01-07T20:44:04Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47051/43198

There are other open Pull requests which might conflict with changes you have proposed:
- File FWCore/Framework/src/EventProcessor.cc modified in PR(s): Print ES module dependences from Tracer #46929
- File FWCore/Framework/src/SubProcess.cc modified in PR(s): Print ES module dependences from Tracer #46929

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

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

cmsbuild · 2025-01-07T20:49:04Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47051/43199

There are other open Pull requests which might conflict with changes you have proposed:
- File FWCore/Framework/src/EventProcessor.cc modified in PR(s): Print ES module dependences from Tracer #46929
- File FWCore/Framework/src/SubProcess.cc modified in PR(s): Print ES module dependences from Tracer #46929

cmsbuild · 2025-01-07T20:49:31Z

A new Pull Request was created by @Dr15Jones for master.

It involves the following packages:

FWCore/Framework (core)
FWCore/TFWLiteSelector (core)
FWCore/TestProcessor (core)
IOPool/Input (core)
IOPool/SecondaryInput (core)

@Dr15Jones, @cmsbuild, @makortel, @smuzaffar can you please review it and eventually sign? Thanks.
@makortel, @missirol, @wddgit this is something you requested to watch as well.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

Dr15Jones · 2025-01-08T22:13:30Z

please test

cmsbuild · 2025-01-09T08:02:36Z

-1

Failed Tests: UnitTests
Size: This PR adds an extra 64KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9e6aa1/43688/summary.html
COMMIT: d501e8c
CMSSW: CMSSW_15_0_X_2025-01-08-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/47051/43688/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found 1 errors in the following unit tests:

---> test runtestPhysicsToolsPatAlgos had ERRORS

Comparison Summary

Summary:

You potentially removed 1 lines from the logs
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3818730
DQMHistoTests: Total failures: 464
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3818246
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 214 log files, 184 edm output root files, 49 DQM output files
TriggerResults: no differences found

cmsbuild · 2025-01-09T13:56:07Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47051/43233

There are other open Pull requests which might conflict with changes you have proposed:
- File DQM/SiStripMonitorHardware/src/SiStripSpyEventMatcher.cc modified in PR(s): [DQM] Apply code checks/format #47002
- File FWCore/Framework/src/EventProcessor.cc modified in PR(s): Print ES module dependences from Tracer #46929, Use WaitingTaskHolder to signal doneWaiting() instead of WaitingTaskWithArenaHolder in framework #47029
- File FWCore/Framework/src/SubProcess.cc modified in PR(s): Print ES module dependences from Tracer #46929

cmsbuild · 2025-01-09T13:56:27Z

Pull request #47051 was updated. @Dr15Jones, @antoniovagnerini, @civanch, @cmsbuild, @kpedro88, @makortel, @mdhildreth, @rseidita, @smuzaffar can you please check and sign again.

Dr15Jones · 2025-01-09T13:59:33Z

please test

cmsbuild · 2025-01-09T17:52:54Z

+1

Size: This PR adds an extra 64KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9e6aa1/43698/summary.html
COMMIT: 91b9436
CMSSW: CMSSW_15_0_X_2025-01-09-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/47051/43698/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-9e6aa1/43698/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9e6aa1/43698/git-merge-result

Comparison Summary

Summary:

You potentially removed 131 lines from the logs
Reco comparison results: 10 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3818730
DQMHistoTests: Total failures: 89
DQMHistoTests: Total nulls: 11
DQMHistoTests: Total successes: 3818610
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.8560000000000004 KiB( 48 files compared)
DQMHistoSizes: changed ( 1000.0,... ): -0.012 KiB MessageLogger/Errors
DQMHistoSizes: changed ( 1000.0,... ): -0.012 KiB MessageLogger/Warnings
DQMHistoSizes: changed ( 24834.911,... ): -0.008 KiB MessageLogger/Errors
DQMHistoSizes: changed ( 24834.911,... ): -0.008 KiB MessageLogger/Warnings
DQMHistoSizes: changed ( 4.22,... ): -0.004 KiB MessageLogger/Errors
DQMHistoSizes: changed ( 4.22,... ): -0.004 KiB MessageLogger/Warnings
Checked 214 log files, 184 edm output root files, 49 DQM output files
TriggerResults: found differences in 1 / 47 workflows

makortel · 2025-01-09T19:32:27Z

DQMHistoTests: Total failures: 89

There seems to be another non-reproducibility in phase2 workflows after #46416 was resolved, I opened #47071

makortel · 2025-01-09T19:32:31Z

+core

civanch · 2025-01-10T17:48:12Z

+1

cmsbuild added this to the CMSSW_15_0_X milestone Jan 7, 2025

cmsbuild added core-pending pending-signatures tests-pending orp-pending code-checks-pending labels Jan 7, 2025

cmsbuild added code-checks-rejected and removed code-checks-pending labels Jan 7, 2025

Dr15Jones commented Jan 7, 2025

View reviewed changes

Dr15Jones force-pushed the productResolverFactory branch from 8e37e06 to 31c0d22 Compare January 7, 2025 20:37

cmsbuild added code-checks-pending and removed code-checks-rejected labels Jan 7, 2025

cmsbuild added code-checks-rejected and removed code-checks-pending labels Jan 7, 2025

Dr15Jones force-pushed the productResolverFactory branch from 31c0d22 to b2ae435 Compare January 7, 2025 20:41

cmsbuild added code-checks-pending and removed code-checks-rejected labels Jan 7, 2025

cmsbuild added code-checks-rejected and removed code-checks-pending labels Jan 7, 2025

Dr15Jones force-pushed the productResolverFactory branch from b2ae435 to 5e50df4 Compare January 7, 2025 20:45

cmsbuild added code-checks-pending and removed code-checks-rejected labels Jan 7, 2025

cmsbuild added code-checks-approved and removed code-checks-pending labels Jan 7, 2025

cmsbuild added tests-started and removed tests-pending labels Jan 8, 2025

cmsbuild added tests-rejected and removed tests-started labels Jan 9, 2025

Moved construction of ProductResolvers to a separate function

91b9436

Dr15Jones force-pushed the productResolverFactory branch from d501e8c to 91b9436 Compare January 9, 2025 13:52

cmsbuild added tests-pending code-checks-pending and removed tests-rejected code-checks-approved labels Jan 9, 2025

cmsbuild added code-checks-approved and removed code-checks-pending labels Jan 9, 2025

cmsbuild added tests-started and removed tests-pending labels Jan 9, 2025

cmsbuild added tests-approved and removed tests-started labels Jan 9, 2025

makortel mentioned this pull request Jan 9, 2025

Non-reproducibility in TrackerPhase2OTL1Track #47071

Open

cmsbuild added core-approved and removed core-pending labels Jan 9, 2025

cmsbuild added simulation-approved and removed simulation-pending labels Jan 10, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moved construction of ProductResolvers to a separate function #47051

Moved construction of ProductResolvers to a separate function #47051

Dr15Jones commented Jan 7, 2025 •

edited

Loading

cmsbuild commented Jan 7, 2025 •

edited

Loading

cmsbuild commented Jan 7, 2025

Dr15Jones Jan 7, 2025

cmsbuild commented Jan 7, 2025

cmsbuild commented Jan 7, 2025

cmsbuild commented Jan 7, 2025

cmsbuild commented Jan 7, 2025

Dr15Jones commented Jan 8, 2025

cmsbuild commented Jan 9, 2025

cmsbuild commented Jan 9, 2025

cmsbuild commented Jan 9, 2025

Dr15Jones commented Jan 9, 2025

cmsbuild commented Jan 9, 2025

makortel commented Jan 9, 2025

makortel commented Jan 9, 2025

civanch commented Jan 10, 2025

Moved construction of ProductResolvers to a separate function #47051

Are you sure you want to change the base?

Moved construction of ProductResolvers to a separate function #47051

Conversation

Dr15Jones commented Jan 7, 2025 • edited Loading

PR description:

PR validation:

cmsbuild commented Jan 7, 2025 • edited Loading

cmsbuild commented Jan 7, 2025

Dr15Jones Jan 7, 2025

Choose a reason for hiding this comment

cmsbuild commented Jan 7, 2025

cmsbuild commented Jan 7, 2025

cmsbuild commented Jan 7, 2025

cmsbuild commented Jan 7, 2025

Dr15Jones commented Jan 8, 2025

cmsbuild commented Jan 9, 2025

Unit Tests

Comparison Summary

cmsbuild commented Jan 9, 2025

cmsbuild commented Jan 9, 2025

Dr15Jones commented Jan 9, 2025

cmsbuild commented Jan 9, 2025

Comparison Summary

makortel commented Jan 9, 2025

makortel commented Jan 9, 2025

civanch commented Jan 10, 2025

Dr15Jones commented Jan 7, 2025 •

edited

Loading

cmsbuild commented Jan 7, 2025 •

edited

Loading