Implement `PathStateCapture` and `PathStateRelease` by fwyzard · Pull Request #48598 · cms-sw/cmssw

fwyzard · 2025-07-21T21:53:13Z

PR description:

An edm::PathStateCapture produces an edm::PathStateToken when run.

The edm::PathStateToken is an empty struct, and acts as a "token"; it is used to communicate the status (active or not) of an edm::Path at a given point where an edm::PathStateCapture module is scheduled:

if the path is active, the module runs and produces the token;
if the path is not active the module does not run or produce the token.

While any product would work, the use of a dedicated empty type is more explicit.

An edm::PathStateToken may be consumed by an edm::PathStateRelease, an
EDFilter with a result of true if the token is present, false otherwise.

PR validation:

The new unit test passes.

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

No backport expected.

cmsbuild · 2025-07-21T21:53:38Z

cms-bot internal usage

cmsbuild · 2025-07-21T21:54:41Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48598/45564

There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/Common/src/classes_def.xml modified in PR(s): SiStripClusterizer, an Alpaka port of the CUDA ClustersFromRawProducerGPU #47629, Migrate LST outputs to Portable SoAs #48409

cmsbuild · 2025-07-21T21:55:05Z

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

It involves the following packages:

DataFormats/Common (core)
FWCore/Modules (core)

@Dr15Jones, @cmsbuild, @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.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

fwyzard · 2025-07-21T22:00:55Z

please test

fwyzard · 2025-07-21T22:01:42Z

@Dr15Jones here are the modules we discussed.
Suggestion for better modules' name would be quite welcome 😁

cmsbuild · 2025-07-21T23:22:48Z

+1

Size: This PR adds an extra 36KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e6f934/47313/summary.html
COMMIT: ec5a788
CMSSW: CMSSW_15_1_X_2025-07-21-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/48598/47313/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: 1
DQMHistoTests: Total histograms compared: 0
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 0
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0 KiB( 0 files compared)
Checked 0 log files, 0 edm output root files, 1 DQM output files

Dr15Jones · 2025-07-22T13:21:07Z

FWCore/Modules/src/PathActivityProbe.cc

@@ -0,0 +1,31 @@
+#include "DataFormats/Common/interface/PathActivityToken.h"


I realize PathActivityProducer may not be the final name, but the file name should match the name of the producer class.

Yes, my bad, I forgot to rename the file.

Dr15Jones · 2025-07-22T13:26:43Z

Suggestion for better modules' name would be quite welcome 😁

I'll try bouncing ideas of Matti once he is in. The important ideas is the Filter is trying to replicate the behavior seen by the Producer. So they form a pair. Maybe using Sender/Receiver in the names to denote the connection?

makortel · 2025-07-22T18:57:41Z

Chris and I brainstormed on the naming and came up with a suggestion

PathActivityToken -> PathCheckpointToken
PathActivityProducer -> PathCheckpoint
PathActivityFilter -> PathCheckpointReplicator

makortel · 2025-07-22T18:58:32Z

DataFormats/Common/src/classes_def.xml

+ <class name="edm::PathActivityToken" ClassVersion="3">
+   <version ClassVersion="3" checksum="4071377827"/>
+ </class>
+ <class name="edm::Wrapper<edm::PathActivityToken>"/>


Can the data product be made transient (wrt OutputModules)

Suggested change

<class name="edm::PathActivityToken" ClassVersion="3">

<version ClassVersion="3" checksum="4071377827"/>

</class>

<class name="edm::Wrapper<edm::PathActivityToken>"/>

<class name="edm::PathActivityToken"/>

<class name="edm::Wrapper<edm::PathActivityToken>" persistent="false"/>

?

No, the idea is to serialise it and transfer it over the network, which requires a dictionary.
The requirement could be bypassed using some spacial-casing - but in general I don't see why we shouldn't support storing this.

Marking the Wrapper transient doesn't preclude ROOT serialization in general. The persistent is a CMS-specific attribute, whose main impact is that OutputModules ignore such data products (plus some knock-on effects in the framework).

How does one check at runtime if a Wrapper has a ROOT dictionary ?

DataFormats/Provenance/interface/BranchDescription.h only provides transient().

But orthogonal to that - why one should not be allowed to serialise this product ?

How does one check at runtime if a Wrapper has a ROOT dictionary ?

Presently the framework requires a dictionary for all data products put into the Event. When we (hopefully) get to the point that transient data products would not need a dictionary, we should provide a clear function to check whether a dictionary exists or not.

But orthogonal to that - why one should not be allowed to serialise this product ?

My line of thought was that if this data product would not have to be stored in files, we could easily avoid a class of mistakes of accidentally storing them (and taking disk space for no reason) by declaring it to be transient. (just to be clear, the "storage in files" is different from general ability to "serialize with ROOT", I wasn't sure which you meant).

But we don't need to argue about this further. I'm fine with allowing the persistency if there is a foreseen use case.

FWCore/Modules/src/PathActivityFilter.cc

FWCore/Modules/src/PathActivityProbe.cc

FWCore/Modules/test/testPathActivityFilter_cfg.py

Dr15Jones · 2025-07-22T20:43:25Z

FWCore/Modules/src/PathActivityFilter.cc

+  class PathActivityFilter : public global::EDFilter<> {
+  public:
+    explicit PathActivityFilter(ParameterSet const& config)
+        : token_(consumes<PathActivityToken>(config.getParameter<edm::InputTag>("producer"))) {}


If you decide to go with Checkpoint in the name, then maybe this should be "checkpoint".

An `edm::PathStateCapture` produces an `edm::PathStateToken` when run. The `edm::PathStateToken` is an empty struct, and acts as a "token"; it is used to communicate the status (active or not) of an `edm::Path` at a given point where an `edm::PathStateCapture` module is scheduled: - if the path is active, the module runs and produces the token; - if the path is not active the module does not run or produce the token. While any product would work, the use of a dedicated empty type is more explicit. An `edm::PathStateToken` may be consumed by an `edm::PathStateRelease`, an `EDFilter` with a result of `true` if the token is present, `false` otherwise.

fwyzard · 2025-07-25T04:17:28Z

please test

cmsbuild · 2025-07-25T04:18:52Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48598/45598

There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/Common/src/classes_def.xml modified in PR(s): SiStripClusterizer, an Alpaka port of the CUDA ClustersFromRawProducerGPU #47629, Migrate LST outputs to Portable SoAs #48409

cmsbuild · 2025-07-25T04:19:13Z

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

fwyzard · 2025-07-25T05:13:57Z

please test

cmsbuild · 2025-07-25T07:29:33Z

+1

Size: This PR adds an extra 36KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e6f934/47384/summary.html
COMMIT: 1511011
CMSSW: CMSSW_15_1_X_2025-07-24-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/48598/47384/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 4 lines to the logs
Reco comparison results: 7 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 4076162
DQMHistoTests: Total failures: 41
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 4076101
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 · 2025-07-25T13:40:10Z

Comparison differences are related to #47071 and #48504

makortel · 2025-07-25T13:40:14Z

+core

cmsbuild · 2025-07-25T13:40:34Z

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

mandrenguyen · 2025-07-26T10:35:37Z

+1

fwyzard · 2025-11-27T07:07:55Z

type ngt

cmsbuild added this to the CMSSW_15_1_X milestone Jul 21, 2025

cmsbuild added core-pending pending-signatures tests-pending orp-pending code-checks-pending changes-dataformats labels Jul 21, 2025

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 21, 2025

cmsbuild added tests-started and removed tests-pending labels Jul 21, 2025

cmsbuild added tests-approved and removed tests-started labels Jul 21, 2025

Dr15Jones reviewed Jul 22, 2025

View reviewed changes

cmsbuild mentioned this pull request Jul 22, 2025

Migrate LST outputs to Portable SoAs #48409

Merged

makortel reviewed Jul 22, 2025

View reviewed changes

Dr15Jones reviewed Jul 22, 2025

View reviewed changes

fwyzard force-pushed the implement_PathActivityFilter branch from ec5a788 to 12670de Compare July 22, 2025 20:44

cmsbuild added tests-pending code-checks-pending and removed tests-approved code-checks-approved labels Jul 22, 2025

cmsbuild added the tests-rejected label Jul 24, 2025

fwyzard force-pushed the implement_PathActivityFilter branch from 701a12b to 1511011 Compare July 25, 2025 04:17

cmsbuild added tests-started code-checks-pending and removed tests-rejected code-checks-approved labels Jul 25, 2025

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 25, 2025

cmsbuild added tests-approved and removed tests-started labels Jul 25, 2025

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Jul 25, 2025

cmsbuild added orp-approved and removed orp-pending labels Jul 26, 2025

cmsbuild merged commit 332067c into cms-sw:master Jul 26, 2025
10 checks passed

fwyzard deleted the implement_PathActivityFilter branch August 13, 2025 15:10

cmsbuild added the ngt label Nov 27, 2025

		@@ -0,0 +1,31 @@
		#include "DataFormats/Common/interface/PathActivityToken.h"

Conversation

fwyzard commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR description:

PR validation:

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

Uh oh!

cmsbuild commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmsbuild commented Jul 21, 2025

Uh oh!

cmsbuild commented Jul 21, 2025

Uh oh!

fwyzard commented Jul 21, 2025

Uh oh!

fwyzard commented Jul 21, 2025

Uh oh!

cmsbuild commented Jul 21, 2025

Comparison Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dr15Jones commented Jul 22, 2025

Uh oh!

makortel commented Jul 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fwyzard commented Jul 25, 2025

Uh oh!

cmsbuild commented Jul 25, 2025

Uh oh!

cmsbuild commented Jul 25, 2025

Uh oh!

fwyzard commented Jul 25, 2025

Uh oh!

cmsbuild commented Jul 25, 2025

Comparison Summary

Uh oh!

makortel commented Jul 25, 2025

Uh oh!

makortel commented Jul 25, 2025

Uh oh!

cmsbuild commented Jul 25, 2025

Uh oh!

mandrenguyen commented Jul 26, 2025

Uh oh!

Uh oh!

fwyzard commented Nov 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fwyzard commented Jul 21, 2025 •

edited

Loading

cmsbuild commented Jul 21, 2025 •

edited

Loading