Skip to content

Determine non-process name lookups at begin job#48807

Merged
cmsbuild merged 13 commits intocms-sw:masterfrom
Dr15Jones:staticNoProcess
Sep 16, 2025
Merged

Determine non-process name lookups at begin job#48807
cmsbuild merged 13 commits intocms-sw:masterfrom
Dr15Jones:staticNoProcess

Conversation

@Dr15Jones
Copy link
Copy Markdown
Contributor

@Dr15Jones Dr15Jones commented Aug 26, 2025

PR description:

When a specific process name is not given during the consumes call, the system now finds the best matching process to use at begin job time. If at transition processing time the data product matched is not found, no further matches are attempted.

Internally the ProductResolverIndexHelper now holds an entry to lookup which ProductResolver to call if one requests to skip current process . This is now also determined at begin job time rather than at run time.

PR validation:

Code compiles and all framework unit tests (after modification) pass.

resolves cms-sw/framework-team#1522

@Dr15Jones
Copy link
Copy Markdown
Contributor Author

please test

@cmsbuild
Copy link
Copy Markdown
Contributor

cmsbuild commented Aug 26, 2025

cms-bot internal usage

@cmsbuild
Copy link
Copy Markdown
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48807/45913

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

@Dr15Jones
Copy link
Copy Markdown
Contributor Author

please test

@cmsbuild
Copy link
Copy Markdown
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48807/45914

@cmsbuild
Copy link
Copy Markdown
Contributor

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

It involves the following packages:

  • DataFormats/Provenance (core)
  • DataFormats/StdDictionaries (core)
  • FWCore/Framework (core)
  • FWCore/Integration (core)
  • FWCore/Sources (core)
  • FWCore/TestProcessor (core)
  • IOPool/Input (core)

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

cms-bot commands are listed here

@cmsbuild
Copy link
Copy Markdown
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48807/45915

An empty ROOT files contains no ProcessHistory which causes problems when filling the ProductRegistry at construction time. Now skip empty files until we find a file with data.
Instead of dynamically determining what data product to get at Run/LuminosityBlock/Event transition time, the data product to get is now determined at begin job time. This is done for no-process and the skip current process cases.
Only call produces and consumes if the data products the module
want appear to be in the job.
This solves the case where the module has been added to a job
just on the remote possibility it might be needed but infact
the values which are actually wanted come from the source.
The MixingModule does not use consumes when getting data from the
mixing files.
@Dr15Jones
Copy link
Copy Markdown
Contributor Author

please test

@cmsbuild
Copy link
Copy Markdown
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48807/46046

@cmsbuild
Copy link
Copy Markdown
Contributor

Pull request #48807 was updated. @civanch, @kpedro88, @lviliani, @mdhildreth, @mkirsano, @ssen, @theofil can you please check and sign again.

@makortel
Copy link
Copy Markdown
Contributor

@cmsbuild, please test

@makortel
Copy link
Copy Markdown
Contributor

unhold

@cmsbuild
Copy link
Copy Markdown
Contributor

+1

Size: This PR adds an extra 20KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ddf0c2/48088/summary.html
COMMIT: 8572541
CMSSW: CMSSW_16_0_X_2025-09-12-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/48807/48088/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially removed 164 lines from the logs
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 4113751
  • DQMHistoTests: Total failures: 3
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 4113728
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
  • Checked 215 log files, 184 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

@makortel
Copy link
Copy Markdown
Contributor

@cms-sw/simulation-l2 @cms-sw/generators-l2 Could you please review and sign? Thanks!

@kpedro88
Copy link
Copy Markdown
Contributor

+simulation

@Dr15Jones
Copy link
Copy Markdown
Contributor Author

@cms-sw/generators-l2 please sign as this is holding up further developments

@lviliani
Copy link
Copy Markdown
Contributor

+generators

@cmsbuild
Copy link
Copy Markdown
Contributor

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

@Dr15Jones
Copy link
Copy Markdown
Contributor Author

@cms-sw/orp-l2 if this could get merged in sooner rather than later it would make it possible to start fixing other PRs which will likely fail to merge because of changing the same files.

@mandrenguyen
Copy link
Copy Markdown
Contributor

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use direct access to correct ProductResolver for 'no process' case

7 participants