Skip to content

[NGT] add e-gamma sequences to NGT scouting menu #47932

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 23, 2025

Conversation

mmusich
Copy link
Contributor

@mmusich mmusich commented Apr 23, 2025

PR description:

After the integration of PR #47901 and the subsequent check of the timing of the NGT scouting menu (integrated back in #47434) it became apparent that none of the e-gamma related parts of the pie were being executed, see e.g. for reference here:

Screenshot from 2025-04-23 08-16-16

The goal of this PR is to introduce said sequences in order to generate the event products foreseen for the NGT scouting.

PR validation:

Run successfully the workflow: runTheMatrix.py --what upgrade -l 29634.77 -t 4 -j 8.

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:

Not a backport, not to be backported.

Cc: @rovere

@mmusich
Copy link
Contributor Author

mmusich commented Apr 23, 2025

type ngt

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 23, 2025

cms-bot internal usage

@mmusich
Copy link
Contributor Author

mmusich commented Apr 23, 2025

test parameters:

  • relvals_opt = --what upgrade
  • workflows = 29634.77
  • enable = hlt_p2_timing

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47932/44587

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • HLTrigger/Configuration (hlt)

@Martin-Grunewald, @cmsbuild, @mmusich can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @SohamBhattacharya, @VourMa, @missirol, @rovere 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

@mmusich
Copy link
Contributor Author

mmusich commented Apr 23, 2025

@cmsbuild, please test

+ hltParticleFlowRecHitECALUnseeded
+ hltParticleFlowClusterECALUncorrectedUnseeded
+ hltParticleFlowClusterECALUnseeded
+ HLTHgcalTiclPFClusteringForEgammaUnseededSequence
+ HLTHgcalTiclPFClusteringForEgamma
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand why we have those two sequences together?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise the rest of the chain fails?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a good answer 😄
What are the differences between the two sequences? And why do we need them both?

Copy link
Contributor Author

@mmusich mmusich Apr 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_HgcalLocalRecoUnseededSequence = cms.Sequence(hltHgcalDigis+hltHGCalUncalibRecHit+hltHGCalRecHit+hltParticleFlowRecHitHGC+hltHgcalLayerClustersEE+hltHgcalLayerClustersHSci+hltHgcalLayerClustersHSi+hltHgcalMergeLayerClusters)
_HgcalTICLPatternRecognitionUnseededSequence = cms.Sequence(hltFilteredLayerClustersCLUE3DHigh+hltTiclSeedingGlobal+hltTiclLayerTileProducer+hltTiclTrackstersCLUE3DHigh)
_SuperclusteringUnseededSequence = cms.Sequence(hltParticleFlowClusterHGCalFromTICLUnseeded+hltParticleFlowSuperClusterHGCalFromTICLUnseeded)
# The baseline sequence
HLTHgcalTiclPFClusteringForEgammaUnseededSequence = cms.Sequence(_HgcalLocalRecoUnseededSequence + _HgcalTICLPatternRecognitionUnseededSequence + _SuperclusteringUnseededSequence)

vs

HLTHgcalTiclPFClusteringForEgamma = cms.Sequence((hltHgcalDigis+hltHGCalUncalibRecHit+hltHGCalRecHit+hltParticleFlowRecHitHGC+hltHgcalLayerClustersEE+hltHgcalLayerClustersHSci+hltHgcalLayerClustersHSi+hltHgcalMergeLayerClusters+hltFilteredLayerClustersCLUE3DHigh+hltTiclSeedingGlobal+hltTiclLayerTileProducer+hltTiclTrackstersCLUE3DHigh+hltParticleFlowClusterHGCal+hltParticleFlowSuperClusterHGCal))

so:

HLTHgcalTiclPFClusteringForEgammaUnseededSequence =  cms.Sequence(
    hltHgcalDigis+
    hltHGCalUncalibRecHit+
    hltHGCalRecHit+
    hltParticleFlowRecHitHGC+
    hltHgcalLayerClustersEE+
    hltHgcalLayerClustersHSci+
    hltHgcalLayerClustersHSi+
    hltHgcalMergeLayerClusters+ 
    hltFilteredLayerClustersCLUE3DHigh+
    hltTiclSeedingGlobal+
    hltTiclLayerTileProducer+
    hltTiclTrackstersCLUE3DHigh+
    hltParticleFlowClusterHGCalFromTICLUnseeded+
    hltParticleFlowSuperClusterHGCalFromTICLUnseeded)

vs

HLTHgcalTiclPFClusteringForEgamma =cms.Sequence(
    hltHgcalDigis+
    hltHGCalUncalibRecHit+
    hltHGCalRecHit+
    hltParticleFlowRecHitHGC+
    hltHgcalLayerClustersEE+
    hltHgcalLayerClustersHSci+
    hltHgcalLayerClustersHSi+
    hltHgcalMergeLayerClusters+
    hltFilteredLayerClustersCLUE3DHigh+
    hltTiclSeedingGlobal+
    hltTiclLayerTileProducer+
    hltTiclTrackstersCLUE3DHigh+
    hltParticleFlowClusterHGCal+
    hltParticleFlowSuperClusterHGCal) 

removing HLTHgcalTiclPFClusteringForEgamma results in:

----- Begin Fatal Exception 23-Apr-2025 10:01:53 CEST-----------------------
An exception of category 'ProductNotFound' occurred while
  [0] Processing  Event run: 1 lumi: 1 event: 3 stream: 0
  [1] Running path 'DST_PFScouting'
  [2] Calling method for module PFClusterRefCandidateProducer/'hltPfClusterRefsForJetsHGCAL'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for a container with elements of type: reco::PFCluster
Looking for module label: hltParticleFlowClusterHGCal
Looking for productInstanceName: 

  Additional Info:
     [a] If you wish to continue processing events after a ProductNotFound exception,
add "TryToContinue = cms.untracked.vstring('ProductNotFound')" to the "options" PSet in the configuration.

----- End Fatal Exception -------------------------------------------------

removing HLTHgcalTiclPFClusteringForEgammaUnseededSequence results in:

----- Begin Fatal Exception 23-Apr-2025 10:06:09 CEST-----------------------
An exception of category 'ProductNotFound' occurred while
  [0] Processing  Event run: 1 lumi: 1 event: 3 stream: 0
  [1] Running path 'DST_PFScouting'
  [2] Calling method for module EgammaHLTRecoEcalCandidateProducers/'hltEgammaCandidatesUnseeded'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for type: std::vector<reco::SuperCluster>
Looking for module label: hltParticleFlowSuperClusterECALUnseeded
Looking for productInstanceName: particleFlowSuperClusterECALBarrel

  Additional Info:
     [a] If you wish to continue processing events after a ProductNotFound exception,
add "TryToContinue = cms.untracked.vstring('ProductNotFound')" to the "options" PSet in the configuration.

----- End Fatal Exception -------------------------------------------------

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It turns out that the sequence HLTHgcalTiclPFClusteringForEgamma was needed only by the other sequence HLTJMESequence, which in turn is apparently used only by a "MC" path (MC_JME):

MC_JME = cms.Path(
HLTBeginSequence
+ HLTRawToDigiSequence
+ HLTHgcalLocalRecoSequence
+ HLTLocalrecoSequence
+ HLTTrackingSequence
+ HLTMuonsSequence
+ HLTParticleFlowSequence
+ HLTHgcalTiclPFClusteringForEgamma
+ HLTJMESequence
+ hltPFPuppiHT
+ hltPFPuppiMHT
)

which we don't want to run in the scouting menu. In the last push I removed the HLTHgcalTiclPFClusteringForEgamma and the HLTJMESequence sequences.

@cmsbuild
Copy link
Contributor

+1

Size: This PR adds an extra 20KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5f91fe/45678/summary.html
COMMIT: 142944c
CMSSW: CMSSW_15_1_X_2025-04-22-2300/el8_amd64_gcc12
Additional Tests: HLT_P2_TIMING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/47932/45678/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 1 lines to the logs
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 10 differences found in the comparisons
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3922941
  • DQMHistoTests: Total failures: 92
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3922829
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
  • Checked 218 log files, 186 edm output root files, 51 DQM output files
  • TriggerResults: found differences in 1 / 49 workflows

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47932/44591

@cmsbuild
Copy link
Contributor

Pull request #47932 was updated. @Martin-Grunewald, @cmsbuild, @mmusich can you please check and sign again.

@mmusich
Copy link
Contributor Author

mmusich commented Apr 23, 2025

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

+1

Size: This PR adds an extra 20KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5f91fe/45683/summary.html
COMMIT: b8e4b2d
CMSSW: CMSSW_15_1_X_2025-04-22-2300/el8_amd64_gcc12
Additional Tests: HLT_P2_TIMING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/47932/45683/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 3 lines to the logs
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3922941
  • DQMHistoTests: Total failures: 46
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3922875
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
  • Checked 218 log files, 186 edm output root files, 51 DQM output files
  • TriggerResults: found differences in 1 / 49 workflows

@mmusich
Copy link
Contributor Author

mmusich commented Apr 23, 2025

For record this is the change in timing:

CMSSW_15_1_X_2025-04-22-2300 CMSSW_15_1_X_2025-04-22-2300 + this PR
Screenshot from 2025-04-23 14-03-47 Screenshot from 2025-04-23 14-03-37

So we go from 6.1851s to 6.0275s i.e. a -2.54% reduction (which we can attribute to the removal of the HLTHgcalTiclPFClusteringForEgamma and the HLTJMESequence sequences).
In the new pie we can notice a contribution from Egamma of about 2% of the timing.

@mmusich
Copy link
Contributor Author

mmusich commented Apr 23, 2025

+hlt

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

@mandrenguyen
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 40514c1 into cms-sw:master Apr 23, 2025
13 checks passed
@mmusich mmusich deleted the mm_dev_add_egamma_NGTScouting branch April 23, 2025 17:06
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.

4 participants