Print ES module dependences from Tracer #46929

wddgit · 2024-12-12T22:16:17Z

PR description:

The Tracer service will print out information about the dependences between modules when the dumpPathsAndConsumes configuration parameter is set true. Before this PR, this information only covered dependences between EDFilters, EDAnalyzers and EDProducers. This PR extends that to also include EventSetup modules. The dependency information it prints is based upon the consumes calls in the C++ code.

Issue #12126 requests this capability. This is an old request, originally from 2015. One weakness of this PR is that the dump is not going to give much insight into what goes on inside PoolDBESSource. Many consumed products will come from that one module.

For the most part, this PR adds and doesn't modify existing code. It shouldn't modify the behavior of the EventSetup. It shouldn't affect output of the program. Mostly it is additional code to get data out of existing data structures that are somewhat difficult to access.

When reviewing this PR, one might benefit by first looking at the following file that contains reference output of a unit test:

FWCore/Integration/test/unit_test_outputs/testConsumesInfo_1.log

There are 5 main parts in the output.

Paths and the modules on paths
ED modules and modules they depend on
EventSetup modules and modules they depend on
ED modules with individual data products they consume and more detail
EventSetup modules with individual data products they consume and more detail

Parts 3 and 5 are new. Parts 2 and 4 have EventSetup dependences added. Note that parts 4 and 5 have detail that includes both consumer and producer transitionID. The transitionID in the EventSetup is the count of the associated setWhatProduced function call in the ESProducer. This might be important because prefetching depends on this and is not done at the level of an entire ESProducer module.

PR validation:

The existing unit test for this dump of information is extended to cover the various EventSetup cases and the output of the dump is compared to a saved reference file known to be correct by manual inspection.

cmsbuild · 2024-12-12T22:16:43Z

cms-bot internal usage

cmsbuild · 2024-12-12T22:18:22Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46929/42995

There are other open Pull requests which might conflict with changes you have proposed:
- File FWCore/Framework/src/Schedule.cc modified in PR(s): Disable ROOT automatic class parsing during finishSchedule() #46858
- File FWCore/Integration/plugins/BuildFile.xml modified in PR(s): Disable ROOT automatic class parsing during finishSchedule() #46858
- File FWCore/Integration/plugins/RunLumiESAnalyzer.cc modified in PR(s): [RFC] descriptions.addDefault -> descriptions.addWithDefaultLabel #44372
- File FWCore/Integration/plugins/RunLumiESSource.cc modified in PR(s): [RFC] descriptions.addDefault -> descriptions.addWithDefaultLabel #44372

cmsbuild · 2024-12-12T22:18:48Z

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

It involves the following packages:

FWCore/Framework (core)
FWCore/Integration (core)
FWCore/ServiceRegistry (core)
FWCore/Services (core)

@Dr15Jones, @cmsbuild, @makortel, @smuzaffar can you please review it and eventually sign? Thanks.
@fwyzard, @makortel, @missirol 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

wddgit · 2024-12-12T22:21:08Z

please test

cmsbuild · 2024-12-13T02:31:24Z

+1

Size: This PR adds an extra 196KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b428b4/43422/summary.html
COMMIT: 66bc924
CMSSW: CMSSW_15_0_X_2024-12-12-1700/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/46929/43422/install.sh to create a dev area with all the needed externals and cmssw changes.

DAS Queries: The DAS query tests failed, see the summary page for details.

Comparison Summary

Summary:

You potentially added 2 lines to the logs
Reco comparison results: 357 differences found in the comparisons
DQMHistoTests: Total files compared: 46
DQMHistoTests: Total histograms compared: 3510017
DQMHistoTests: Total failures: 6102
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3503895
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 45 files compared)
Checked 202 log files, 172 edm output root files, 46 DQM output files
TriggerResults: no differences found

makortel

Comments from a first pass

makortel · 2024-12-13T16:49:26Z

FWCore/Framework/interface/EventSetupRecordImpl.h

+      ComponentDescription const* getComponentDescription(ESResolverIndex iResolverIndex) const;
+      unsigned int getTransitionID(ESResolverIndex iResolverIndex) const;


What would you think about dropping the get prefix?

Suggested change

ComponentDescription const* getComponentDescription(ESResolverIndex iResolverIndex) const;

unsigned int getTransitionID(ESResolverIndex iResolverIndex) const;

ComponentDescription const* componentDescription(ESResolverIndex iResolverIndex) const;

unsigned int transitionID(ESResolverIndex iResolverIndex) const;

makortel · 2024-12-13T17:00:52Z

FWCore/ServiceRegistry/interface/EventSetupConsumesInfo.h

+namespace edm {
+  class EventSetupConsumesInfo {
+  public:
+    EventSetupConsumesInfo(std::string_view const& iEventSetupRecordType,


string_view can be considered as a lightweight type that would be better passed by value

Suggested change

EventSetupConsumesInfo(std::string_view const& iEventSetupRecordType,

EventSetupConsumesInfo(std::string_view iEventSetupRecordType,

makortel · 2024-12-13T17:09:49Z

FWCore/Integration/plugins/ReadAndCopyWhatsIt.cc

+     Produces new WhatsIt products for the EventSetup by copying
+     the products it read.


To me it would be more clear to describe already here that the input ES data products are not actually copied.

makortel · 2024-12-13T17:19:12Z

FWCore/Framework/interface/CallbackProductResolver.h

@@ -75,6 +75,8 @@ namespace edm::eventsetup {
    CallbackProductResolver(const CallbackProductResolver&) = delete;
    const CallbackProductResolver& operator=(const CallbackProductResolver&) = delete;

+    unsigned int transitionID() const final { return callback_->transitionID(); }


This is the "counter" for the setWhatProduced() calls in the ESProducer, right? I was going to suggest if we could think of a better word (I can see potential confusion with the framework transitions), but I see we already use the transitionID() term for this purpose.

Yes. It is the counter for the setWhatProduced calls in ESProducer. Adding this function to the resolver interface does make the name more visible, but we were already using this name. The transitionID function in CallbackBase was already there and this PR didn't modify that. It also was present in several other files already.

I could change it. We could also leave that for a separate PR. But I can't think of a better name and in some places it is convenient to use the same name as we use in EDConsumerBase. But I agree it leads to some confusion. It has been that way a while.

makortel · 2024-12-13T17:24:11Z

FWCore/ServiceRegistry/interface/ActivityRegistry.h

@@ -197,6 +197,16 @@ namespace edm {
    void watchPostEndJob(PostEndJob::slot_type const& iSlot) { postEndJobSignal_.connect_front(iSlot); }
    AR_WATCH_USING_METHOD_0(watchPostEndJob)

+    typedef signalslot::Signal<void(PathsAndConsumesOfModulesBase const&, ProcessContext const&)>


What is the state of PathsAndConsumesOfModuleBase object in the existing PreBeginJob signal?

makortel · 2024-12-13T19:49:06Z

FWCore/ServiceRegistry/interface/PathsAndConsumesOfModulesBase.h

-    // that is held in memory throughout the job, while the following
-    // function returns a newly created object each time.  We do not
-    // expect this to be called during a normal production job where
+    std::vector<eventsetup::ComponentDescription const*> const& esModulesWhoseProductsAreConsumedBy(


Could you add a comment describing the return value in similar way to the modulesWhoseProductsAreConsumedBy() above?

makortel · 2024-12-13T19:58:17Z

One weakness of this PR is that the dump is not going to give much insight into what goes on inside PoolDBESSource.

Do you mean whatever PoolDBESSource itself does is not shown? But the dependencies from other (ES or ED) modules to ES data products produced by PoolDBESSource are included in the dump?

makortel · 2024-12-13T19:58:56Z

Comparison differences are related to #39803 and #46416

wddgit · 2024-12-18T19:56:56Z

I noticed that I neglected to implement code to handle the "may consumes" case even though I had thought about it and had a plan for how to do it. I am working on that now. It is mostly just additions to what is already here, not much significant changes in the existing PR code. I'll put it into a separate commit. This version just silently ignores those cases. I need to add a test module using "may consumes". Our current test relies on a lower level unit test.

Print ES module dependences from Tracer

66bc924

cmsbuild added this to the CMSSW_15_0_X milestone Dec 12, 2024

cmsbuild added core-pending pending-signatures tests-pending orp-pending code-checks-pending labels Dec 12, 2024

wddgit mentioned this pull request Dec 12, 2024

Determine which modules depend on which conditions #12126

Open

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 12, 2024

cmsbuild added tests-started and removed tests-pending labels Dec 12, 2024

makortel mentioned this pull request Dec 12, 2024

Enhance Tracer service to print out EventSetup consumes information cms-sw/framework-team#448

Open

cmsbuild added tests-approved and removed tests-started labels Dec 13, 2024

makortel reviewed Dec 13, 2024

View reviewed changes

cmsbuild mentioned this pull request Dec 18, 2024

Disable ROOT automatic class parsing during finishSchedule() #46858

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Print ES module dependences from Tracer #46929

Print ES module dependences from Tracer #46929

wddgit commented Dec 12, 2024

cmsbuild commented Dec 12, 2024 •

edited

Loading

cmsbuild commented Dec 12, 2024

cmsbuild commented Dec 12, 2024

wddgit commented Dec 12, 2024

cmsbuild commented Dec 13, 2024

makortel left a comment

makortel Dec 13, 2024

makortel Dec 13, 2024

makortel Dec 13, 2024

makortel Dec 13, 2024

wddgit Dec 16, 2024

makortel Dec 13, 2024

makortel Dec 13, 2024

makortel commented Dec 13, 2024

makortel commented Dec 13, 2024

wddgit commented Dec 18, 2024

		ComponentDescription const* getComponentDescription(ESResolverIndex iResolverIndex) const;
		unsigned int getTransitionID(ESResolverIndex iResolverIndex) const;

	EventSetupConsumesInfo(std::string_view const& iEventSetupRecordType,
	EventSetupConsumesInfo(std::string_view iEventSetupRecordType,

		Produces new WhatsIt products for the EventSetup by copying
		the products it read.

Print ES module dependences from Tracer #46929

Are you sure you want to change the base?

Print ES module dependences from Tracer #46929

Conversation

wddgit commented Dec 12, 2024

PR description:

PR validation:

cmsbuild commented Dec 12, 2024 • edited Loading

cmsbuild commented Dec 12, 2024

cmsbuild commented Dec 12, 2024

wddgit commented Dec 12, 2024

cmsbuild commented Dec 13, 2024

Comparison Summary

makortel left a comment

Choose a reason for hiding this comment

makortel Dec 13, 2024

Choose a reason for hiding this comment

makortel Dec 13, 2024

Choose a reason for hiding this comment

makortel Dec 13, 2024

Choose a reason for hiding this comment

makortel Dec 13, 2024

Choose a reason for hiding this comment

wddgit Dec 16, 2024

Choose a reason for hiding this comment

makortel Dec 13, 2024

Choose a reason for hiding this comment

makortel Dec 13, 2024

Choose a reason for hiding this comment

makortel commented Dec 13, 2024

makortel commented Dec 13, 2024

wddgit commented Dec 18, 2024

cmsbuild commented Dec 12, 2024 •

edited

Loading