Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update the autoSkim.py for the Run3 skims #38479

Merged
merged 1 commit into from
Jul 18, 2022
Merged

Update the autoSkim.py for the Run3 skims #38479

merged 1 commit into from
Jul 18, 2022

Conversation

sam7k9621
Copy link
Contributor

@sam7k9621 sam7k9621 commented Jun 23, 2022 •
edited
Loading

PR description:

This PR is to update the autoSkim for the Run3 skims.
Following the discussion in #37656, the autoSkim is now for Run3.

PR validation:

None

if this PR is a backport please specify the original PR and why you need to backport that PR:

backport of #37655

@cmsbuild cmsbuild added this to the CMSSW_12_5_X milestone Jun 23, 2022
@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38479/30695

  • This PR adds an extra 12KB to repository

@cmsbuild
Copy link
Contributor

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 you are the release manager for this.

cms-bot commands are listed here

@qliphy
Copy link
Contributor

qliphy commented Jul 1, 2022

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 1, 2022

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-929701/25925/summary.html
COMMIT: 9b7b351
CMSSW: CMSSW_12_5_X_2022-06-30-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38479/25925/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: 8 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 3654771
  • DQMHistoTests: Total failures: 13
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 3654735
  • 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

@jordan-martins
Copy link
Contributor

+pdmv

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

@qliphy
Copy link
Contributor

qliphy commented Jul 18, 2022

+1

@cmsbuild cmsbuild merged commit 8854324 into cms-sw:master Jul 18, 2022
@cmsbuild cmsbuild mentioned this pull request Jul 18, 2022
Merged
tvami
tvami reviewed Jul 18, 2022
View reviewed changes
Configuration/Skimming/python/autoSkim.py
Comment on lines +17 to +19
'DoubleMuonLowMass' : 'LogError+LogErrorMonitor',
'MuOnia' : 'LogError+LogErrorMonitor',
'Charmonium' : 'MuonPOGJPsiSkim+LogError+LogErrorMonitor',
Copy link
Contributor

Choose a reason for hiding this comment

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

These dont exist anymore

tvami
tvami reviewed Jul 18, 2022
View reviewed changes
Configuration/Skimming/python/autoSkim.py
'DisplacedJet' : 'LogError+LogErrorMonitor',
'MET' : 'HighMET+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
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesnt exists since 2018

tvami
tvami reviewed Jul 18, 2022
View reviewed changes
Configuration/Skimming/python/autoSkim.py
'SinglePhoton' : 'SinglePhotonJetPlusHOFilter+EXOMONOPOLE+LogError+LogErrorMonitor', #to be updated if we will have EGamma as Run-2 (2018), or splitting as 2016,2017
'DoubleEG' : 'ZElectron+LogError+LogErrorMonitor', #to be updated if we will have EGamma as Run-2 (2018), or splitting as 2016,2017
'EGamma':'SinglePhotonJetPlusHOFilter+ZElectron+EXOMONOPOLE+LogError+LogErrorMonitor',
'SinglePhoton' : 'EXOMONOPOLE+LogError+LogErrorMonitor', #to be updated if we will have EGamma as Run-2 (2018), or splitting as 2016,2017
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing here

@tvami
Copy link
Contributor

tvami commented Jul 18, 2022

Also what's the backport of this that entered 12_4_3 ?

@sam7k9621
Copy link
Contributor Author

Also what's the backport of this that entered 12_4_3 ?

There isn't one, since it was just merged into the central CMSSW (12_5_X).
Do we need a backport to 12_4_3?
The current backport refers to 12_4_X.

@tvami
Copy link
Contributor

tvami commented Jul 19, 2022

The current backport refers to 12_4_X.

Which is that backport? This PR is towards 12_5_X, I'm asking which is the version of this PR that's towards 12_4_X. It's certainly not #37655 , right?

@kskovpen
Copy link
Contributor

Hi @sam7k9621 , could you please prepare a backport for 12_4_X?

@sam7k9621
Copy link
Contributor Author

Hi @kskovpen and @tvami ,
I am now preparing the backport for 12_4_X and also a new PR to address the comment (removing the unused PD).

This was referenced Jul 20, 2022
Merged
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tvami tvami tvami left review comments

Assignees
No one assigned
Labels
code-checks-approved fully-signed orp-approved pdmv-approved tests-approved
Projects
None yet
Milestone
CMSSW_12_5_X
Development

Successfully merging this pull request may close these issues.
6 participants
@sam7k9621 @cmsbuild @qliphy @jordan-martins @tvami @kskovpen