Update the autoSkim.py for the Run3 skims and removed unused PD #38792

sam7k9621 · 2022-07-20T07:46:15Z

PR description:

Addressed the comments in #38479
12_4_X backport (#38795)

PR validation:

None

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:

cmsbuild · 2022-07-20T07:46:34Z

@sam7k9621, CMSSW_12_5_X branch is closed for direct updates. cms-bot is going to move this PR to master branch.
In future, please use cmssw master branch to submit your changes.

cmsbuild · 2022-07-20T07:57:47Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38792/31148

This PR adds an extra 16KB to repository

cmsbuild · 2022-07-20T07:58:12Z

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

It involves the following packages:

Configuration/Skimming (pdmv)

@cmsbuild, @bbilin, @kskovpen, @jordan-martins can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @missirol, @fabiocos this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy, @rappoccio you are the release manager for this.

cms-bot commands are listed here

tvami · 2022-07-20T07:58:40Z

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:
None

@sam7k9621 this is not true, we need these things in 12_4_X -- that is the release for data taking

cmsbuild · 2022-07-20T08:04:17Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38792/31149

This PR adds an extra 16KB to repository

tvami · 2022-07-20T08:05:49Z

Configuration/Skimming/python/autoSkim.py

 'BTagMu' : 'LogError+LogErrorMonitor',
- 'HTMHT' : 'LogError+LogErrorMonitor',
 'JetHT' : 'JetHTJetPlusHOFilter+LogError+LogErrorMonitor',
 'DisplacedJet' : 'EXODisplacedJet+EXODelayedJet+EXODTCluster+EXOCSCCluster+LogError+LogErrorMonitor',


T0 only has

"EXODisplacedJet", "LogError", "LogErrorMonitor"

so EXODelayedJet+EXODTCluster+EXOCSCCluster needs to be added for Run-3 from T0, is that correct?

Yes, those are the new skims for Run3.
However, EXOCSCCluster has not yet been merged (#37782)
I have already notified the L2 convener to review the PR

However, EXOCSCCluster has not yet been merged

So these are the real kind of problems... How is expected to be used in the data taking with 12_4_X when the 12_5_X is not even merged? Why is #37782 not made urgent? Is there a request from PdmV to have a new release which includes this? (These are not really questions to you @sam7k9621 but more @bbilin @kskovpen @rappoccio )

We will make it urgent

tvami · 2022-07-20T08:07:00Z

Configuration/Skimming/python/autoSkim.py

 'BTagMu' : 'LogError+LogErrorMonitor',
- 'HTMHT' : 'LogError+LogErrorMonitor',
 'JetHT' : 'JetHTJetPlusHOFilter+LogError+LogErrorMonitor',
 'DisplacedJet' : 'EXODisplacedJet+EXODelayedJet+EXODTCluster+EXOCSCCluster+LogError+LogErrorMonitor',
 'MET' : 'EXOHighMET+EXODelayedJetMET+LogError+LogErrorMonitor',


T0 only has

"EXOMONOPOLE", "HighMET", "LogError", "LogErrorMonitor"

so EXOHighMET+EXODelayedJetMET should be added, and EXOMONOPOLE to be removed from T0, correct?

Yes, those two skims are new for Run3
And the monopole skim has been updated to EGamma PD

tvami · 2022-07-20T08:08:48Z

Configuration/Skimming/python/autoSkim.py

 'JetHT' : 'JetHTJetPlusHOFilter+LogError+LogErrorMonitor',
 'DisplacedJet' : 'EXODisplacedJet+EXODelayedJet+EXODTCluster+EXOCSCCluster+LogError+LogErrorMonitor',
 'MET' : 'EXOHighMET+EXODelayedJetMET+LogError+LogErrorMonitor',
- 'SingleElectron' : 'LogError+LogErrorMonitor', #to be updated if we will have EGamma as Run-2 (2018), or splitting as 2016,2017
- 'SinglePhoton' : 'EXOMONOPOLE+LogError+LogErrorMonitor', #to be updated if we will have EGamma as Run-2 (2018), or splitting as 2016,2017
- 'DoubleEG' : 'LogError+LogErrorMonitor', #to be updated if we will have EGamma as Run-2 (2018), or splitting as 2016,2017
 'EGamma':'ZElectron+WElectron+EXOMONOPOLE+LogError+LogErrorMonitor',
 'Tau' : 'LogError+LogErrorMonitor',
 'SingleMuon' : 'ZMu+LogError+LogErrorMonitor',


T0 has

"MuonPOGSkim", "MuTau", "ZMu", "LogError", "LogErrorMonitor"

so should MuonPOGSkim, MuTau be removed from T0?

Yes, those skims are dropped during Run3

tvami · 2022-07-20T08:23:10Z

T0 also has
DoubleMuonLowPU with "LogError", "LogErrorMonitor"
ParkingDoubleMuonLowMass0 with "LogError", "LogErrorMonitor", "BPHSkim", "MuonPOGJPsiSkim"
HLTPhysics* with "LogError", "LogErrorMonitor"

are these not needed anymore?

sam7k9621 · 2022-07-20T08:34:48Z

T0 also has DoubleMuonLowPU with "LogError", "LogErrorMonitor" ParkingDoubleMuonLowMass0 with "LogError", "LogErrorMonitor", "BPHSkim", "MuonPOGJPsiSkim" HLTPhysics* with "LogError", "LogErrorMonitor"

are these not needed anymore?

I didn't see DoubleMuonLowPU and ParkingDoubleMuonLowMass0 in the PD list for run3. (https://github.com/cms-sw/cmssw/blob/CMSSW_10_6_X/HLTrigger/Configuration/python/HLTrigger_Datasets_GRun_cff.py)
We still keep the HLTPhysics.

tvami · 2022-07-20T08:45:51Z

I didn't see DoubleMuonLowPU and ParkingDoubleMuonLowMass0 in the PD list for run3. (https://github.com/cms-sw/cmssw/blob/CMSSW_10_6_X/HLTrigger/Configuration/python/HLTrigger_Datasets_GRun_cff.py)

Why are you looking at 10_6_X ?

We still keep the HLTPhysics.

So maybe time to add it to the autoSkim file too?

Please also adress my comments in the code review

sam7k9621 · 2022-07-20T08:46:42Z

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:
None

@sam7k9621 this is not true, we need these things in 12_4_X -- that is the release for data taking

Hi @tvami , I have added the backport toward 12_4_X

tvami · 2022-07-20T09:31:49Z

PR validation:
None

Are there any workflows that actually run these skims btw?

tvami · 2022-07-20T09:36:21Z

Hi @tvami , I have added the backport toward 12_4_X

Thanks, please include its number in the PR description

cmsbuild · 2022-07-20T13:08:12Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-572a3e/26337/summary.html
COMMIT: 5085c1a
CMSSW: CMSSW_12_5_X_2022-07-19-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/38792/26337/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: 6 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3662417
DQMHistoTests: Total failures: 13
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3662381
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 49 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 208 log files, 45 edm output root files, 50 DQM output files
TriggerResults: no differences found

tvami · 2022-07-20T13:10:39Z

cmsbuild commented 1 minute ago
+1

ok so apparently nothing really tests the autoSkim content
given the fact that EXOCSCCluster doesnt exists in this IB, a wf that tests the autoSkim should have failed

sam7k9621 · 2022-07-20T13:18:10Z

Hi @kskovpen , do you know which workflow we can test with the autoSkim?

kskovpen · 2022-07-20T20:58:46Z

Hi @kskovpen , do you know which workflow we can test with the autoSkim?

Unless I am mistaken, such workflows never existed. I wonder if we can try to integrate autoSkim within runTheMatrix.py.

tvami · 2022-07-21T08:21:07Z

I wonder if we can try to integrate autoSkim within runTheMatrix.py.

Yes it can be done, in AlCa we have the AlCaRECO matrix that's the same kind of object (PD --> AlCaRECO)

cmssw/Configuration/AlCa/python/autoAlca.py

Lines 1 to 34 in c850e19

    
           AlCaRecoMatrix = { 
        
                             "ALCALumiPixelsCountsExpress" : "AlCaPCCRandom", 
        
                             "AlCaLumiPixelsCountsPrompt"  : "AlCaPCCZeroBias+RawPCCProducer", 
        
                             # These two (AlCaPhiSym, AlCaP0) cannot run on RAW, they are just meant to run on the dedicated AlcaRAW so they do not enter the allForPrompt list 
        
                             "AlCaPhiSym"                  : "", 
        
                             "AlCaP0"                      : "", 
        
                             "ALCAPPS"                     : "PPSCalMaxTracks", # Express producer 
        
                             "AlCaPPS"                     : "PPSCalMaxTracks", # Prompt  producer 
        
                             "Charmonium"                  : "TkAlJpsiMuMu", 
        
                             "Commissioning"               : "HcalCalIsoTrk+HcalCalIsolatedBunchSelector+TkAlMinBias+SiStripCalMinBias", 
        
                             "Cosmics"                     : "SiPixelCalCosmics+SiStripCalCosmics+TkAlCosmics0T+MuAlGlobalCosmics", 
        
                             "DoubleMuon"                  : "TkAlZMuMu+TkAlDiMuonAndVertex+MuAlCalIsolatedMu", 
        
                             "DoubleMuonLowMass"           : "TkAlJpsiMuMu+TkAlUpsilonMuMu",  
        
                             "DoubleMuParked"              : "MuAlCalIsolatedMu+MuAlOverlaps+TkAlZMuMu", 
        
                             "EGamma"                      : "EcalESAlign+EcalUncalWElectron+EcalUncalZElectron+HcalCalIsoTrkProducerFilter+HcalCalIterativePhiSym", 
        
                             "Express"                     : "SiStripCalZeroBias+TkAlMinBias+SiStripPCLHistos+SiStripCalMinBias+SiStripCalMinBiasAAG+Hotline+SiPixelCalZeroBias", 
        
                             "ExpressAlignment"            : "TkAlMinBias", 
        
                             "ExpressCosmics"              : "SiStripPCLHistos+SiStripCalZeroBias+TkAlCosmics0T+SiPixelCalZeroBias", 
        
                             "HcalNZS"                     : "HcalCalMinBias", 
        
                             "HLTPhysics"                  : "TkAlMinBias", 
        
                             "JetHT"                       : "HcalCalIsoTrkProducerFilter+TkAlJetHT", 
        
                             "MET"                         : "HcalCalNoise", 
        
                             "MinimumBias"                 : "SiStripCalZeroBias+SiStripCalMinBias+TkAlMinBias", 
        
                             "MuOnia"                      : "TkAlUpsilonMuMu", 
        
                             "MuOniaParked"                : "TkAlJpsiMuMu+TkAlUpsilonMuMu", 
        
                             "NoBPTX"                      : "TkAlCosmicsInCollisions", 
        
                             "SingleMuon"                  : "SiPixelCalSingleMuonLoose+SiPixelCalSingleMuonTight+TkAlMuonIsolated+MuAlCalIsolatedMu+HcalCalHO+HcalCalIterativePhiSym+HcalCalHBHEMuonProducerFilter", 
        
                             "StreamExpress"               : "SiStripCalZeroBias+TkAlMinBias+SiStripPCLHistos+SiStripCalMinBias+SiStripCalMinBiasAAG+Hotline+SiPixelCalZeroBias+SiPixelCalSingleMuon+PPSCalTrackBasedSel", 
        
                             "StreamExpressHI"             : "SiStripCalZeroBias+TkAlMinBiasHI+SiStripPCLHistos+SiStripCalMinBias+SiStripCalMinBiasAAG+SiPixelCalZeroBias", 
        
                             # These (TestEnablesTracker, TestEnablesEcalHcal) are in the AlCaRecoMatrix, but no RelVals are produced 
        
                             # 'TestEnablesTracker'        : 'TkAlLAS' 
        
                             # 'TestEnablesEcalHcal'       : 'HcalCalPedestal' 
        
                             "ZeroBias"                    : "SiStripCalZeroBias+TkAlMinBias+SiStripCalMinBias", 
        
                             }

here it's defined to have them all in for allForPrompt

cmssw/Configuration/AlCa/python/autoAlca.py

Line 53 in c850e19

    
           autoAlca = { 'allForPrompt'         : buildList(['Charmonium', 'Commissioning', 'DoubleMuParked', 'DoubleMuon', 'DoubleMuonLowMass', 'EGamma', 'HLTPhysics', 'HcalNZS', 'JetHT', 'MET', 'MinimumBias', 'MuOnia', 'MuOniaParked', 'NoBPTX', 'SingleMuon', 'ZeroBias'], AlCaRecoMatrix),

which then is applied in a RelVal
https://github.com/tocheng/cmssw/blob/96482c8d51ea17bd11aa39362c6de25c499923d1/Configuration/PyReleaseValidation/python/relval_steps.py#L2085

tvami · 2022-07-21T08:22:12Z

@sam7k9621

I'd like to invite you to join the Joint Ops meeting this Friday:
https://cms-talk.web.cern.ch/t/weekly-join-ops-meeting-tomorrow-friday-22-july-at-14-00/13136

In the gDoc there, please report the summary of this PR under "PDMV" bullet point. Thanks!

tvami · 2022-07-21T12:30:57Z

@kskovpen will you sign this, or you'd like the relval to happen first? Ideally the backport of this should be merged by next Tuesday, so whichever is more realistic

tvami · 2022-07-21T12:31:28Z

I'd like to invite you to join the Joint Ops meeting this Friday:

@sam7k9621 please confirm that you can make this, thanks

kskovpen · 2022-07-21T12:45:29Z

@kskovpen will you sign this, or you'd like the relval to happen first? Ideally the backport of this should be merged by next Tuesday, so whichever is more realistic

A dedicated implementation of skim checks in relval wfs is expected to come later. It would be good to have it, finally.

kskovpen · 2022-07-21T12:45:40Z

+pdmv

cmsbuild · 2022-07-21T12:45:48Z

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

sam7k9621 · 2022-07-21T12:53:47Z

I'd like to invite you to join the Joint Ops meeting this Friday:

@sam7k9621 please confirm that you can make this, thanks

Hi yes I would attend

perrotta · 2022-07-21T13:03:51Z

+1

I understand that some more work is needed (and expected) for those skims
However, this PR just removes a few of them: it can therefore get merged independently from those further developments waited for

tvami · 2022-07-22T14:01:53Z

Hi @sam7k9621
I think this line
https://github.com/sam7k9621/cmssw/blob/5085c1a2bd4cf7d09de3b8a18207f43d47410078/Configuration/Skimming/python/autoSkim.py#L17

should not be there for Run-3. Can you please confirm?

tvami · 2022-07-22T14:11:16Z

Changes propagated to T0 in dmwm/T0#4722

sam7k9621 · 2022-07-22T14:14:04Z

Hi @sam7k9621 I think this line https://github.com/sam7k9621/cmssw/blob/5085c1a2bd4cf7d09de3b8a18207f43d47410078/Configuration/Skimming/python/autoSkim.py#L17

should not be there for Run-3. Can you please confirm?

You are right, it should be removed.
Should I apply for another PR now or change it next week with the new HLT menu (v1.3)?

tvami · 2022-07-22T14:17:08Z

Should I apply for another PR now or change it next week with the new HLT menu (v1.3)?

I think it's fine to do it when you deal with the v1.3 situation. On the other hand, you could also already start to do that, and get it merged in master, the point is that it should not get merged in the backport yet (the master it's fine any day)

Update the autoSkim.py for the Run3 skims and removed unused PD

2a3b3dc

cmsbuild changed the base branch from CMSSW_12_5_X to master July 20, 2022 07:46

cmsbuild added this to the CMSSW_12_5_X milestone Jul 20, 2022

cmsbuild added code-checks-pending orp-pending pdmv-pending pending-signatures tests-pending labels Jul 20, 2022

Update the autoSkim.py for the Run3 skims and removed unused PD

84a5e5e

sam7k9621 mentioned this pull request Jul 20, 2022

Skim pd #38793

Closed

Update the autoSkim.py for the Run3 skims and removed unused PD

5085c1a

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 20, 2022

cmsbuild added code-checks-pending and removed code-checks-approved labels Jul 20, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 20, 2022

tvami reviewed Jul 20, 2022

View reviewed changes

sam7k9621 mentioned this pull request Jul 20, 2022

[12_4_X] Update the autoSkim.py for the Run3 skims and removed unused PD #38795

Merged

cmsbuild added tests-approved and removed tests-started labels Jul 20, 2022

cmsbuild added fully-signed pdmv-approved and removed pending-signatures pdmv-pending labels Jul 21, 2022

cmsbuild added orp-approved and removed orp-pending labels Jul 21, 2022

cmsbuild merged commit 5984a2e into cms-sw:master Jul 21, 2022

cmsbuild mentioned this pull request Jul 21, 2022

[DQM] Fix for DQMStore setting assertLegacySafe to false #38821

Merged

tvami mentioned this pull request Jul 22, 2022

Update PDs and skims for Run-3 dmwm/T0#4722

Merged

Update the autoSkim.py for the Run3 skims and removed unused PD #38792

Update the autoSkim.py for the Run3 skims and removed unused PD #38792

Conversation

sam7k9621 commented Jul 20, 2022 • edited

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:

cmsbuild commented Jul 20, 2022

cmsbuild commented Jul 20, 2022

cmsbuild commented Jul 20, 2022

tvami commented Jul 20, 2022

cmsbuild commented Jul 20, 2022

tvami Jul 20, 2022 • edited

Choose a reason for hiding this comment

sam7k9621 Jul 20, 2022

Choose a reason for hiding this comment

tvami Jul 20, 2022

Choose a reason for hiding this comment

kskovpen Jul 20, 2022

Choose a reason for hiding this comment

tvami Jul 20, 2022 • edited

Choose a reason for hiding this comment

sam7k9621 Jul 20, 2022

Choose a reason for hiding this comment

tvami Jul 20, 2022 • edited

Choose a reason for hiding this comment

sam7k9621 Jul 20, 2022

Choose a reason for hiding this comment

tvami commented Jul 20, 2022 • edited

sam7k9621 commented Jul 20, 2022 • edited

tvami commented Jul 20, 2022

sam7k9621 commented Jul 20, 2022

tvami commented Jul 20, 2022

tvami commented Jul 20, 2022 • edited

cmsbuild commented Jul 20, 2022

Comparison Summary

tvami commented Jul 20, 2022

sam7k9621 commented Jul 20, 2022

kskovpen commented Jul 20, 2022

tvami commented Jul 21, 2022

tvami commented Jul 21, 2022

tvami commented Jul 21, 2022

tvami commented Jul 21, 2022

kskovpen commented Jul 21, 2022

kskovpen commented Jul 21, 2022

cmsbuild commented Jul 21, 2022

sam7k9621 commented Jul 21, 2022

perrotta commented Jul 21, 2022

tvami commented Jul 22, 2022

tvami commented Jul 22, 2022

sam7k9621 commented Jul 22, 2022

tvami commented Jul 22, 2022

sam7k9621 commented Jul 20, 2022 •

edited

tvami Jul 20, 2022 •

edited

tvami Jul 20, 2022 •

edited

tvami Jul 20, 2022 •

edited

tvami commented Jul 20, 2022 •

edited

sam7k9621 commented Jul 20, 2022 •

edited

tvami commented Jul 20, 2022 •

edited