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.

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

Remove hardcoded trigger bits from Lumi ALCARECOs #39519

Merged
merged 3 commits into from
Oct 12, 2022
Merged

Remove hardcoded trigger bits from Lumi ALCARECOs #39519

merged 3 commits into from
Oct 12, 2022

Conversation

Moanwar
Copy link
Contributor

@Moanwar Moanwar commented Sep 28, 2022

PR description:

Remove hardcoded trigger bits from AlCaRecos and include it as a key in the alcareco trigger bits tag in "ALCARECOAlCaPCCZeroBias" and "ALCARECOAlCaPCCRandom"

Two GTs were created to be included in this PR, with the correct AlCaRecoTriggerBits tag.
1- 124X_dataRun3_Express_frozen_v6 :
GlobalTag for Run3 data relvals (express GT) - identical to 124X_dataRun3_Express_v6 but with snapshot at 2022-10-04 14:22:26 (UTC)
Here is the link of difference of GT, old vs new : https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/124X_dataRun3_Express_frozen_v4/124X_dataRun3_Express_frozen_v6

2- 124X_dataRun3_Prompt_frozen_v5 :
GlobalTag for Run3 data relvals (prompt GT) - identical to 124X_dataRun3_Prompt_v5 but with snapshot at 2022-10-04 14:19:51 (UTC)
Here is the link of difference of GT, old vs new : https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/124X_dataRun3_Prompt_frozen_v4/124X_dataRun3_Prompt_frozen_v5

PR validation:

Not a backport, but will be backported for data-taking operations

@cms-AlCaDB/alca-tsg

@cmsbuild cmsbuild changed the base branch from CMSSW_12_6_X to master September 28, 2022 07:00
@cmsbuild
Copy link
Contributor

@Moanwar, CMSSW_12_6_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.

@Moanwar
Copy link
Contributor Author

Moanwar commented Sep 28, 2022

adding @tvami , @francescobrivio

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39519/32278

  • This PR adds an extra 12KB to repository

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • Calibration/LumiAlCaRecoProducers (alca)

@malbouis, @yuanchao, @cmsbuild, @saumyaphor4252, @francescobrivio, @ChrisMisan, @tvami can you please review it and eventually sign? Thanks.
@mmusich, @tocheng this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

HLTPaths = cms.vstring("AlCa_LumiPixelsCounts_Random_v*"),
eventSetupPathsKey='',
#HLTPaths = cms.vstring("AlCa_LumiPixelsCounts_Random_v*"),
eventSetupPathsKey='AlCa_LumiPixelsCounts_Random_v*',
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont think this is the name of the key, is it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
eventSetupPathsKey='AlCa_LumiPixelsCounts_Random_v*',
eventSetupPathsKey='AlCaPCCRandom',

I think this is what we need and then a new trigger bit in the GT

HLTPaths = cms.vstring("AlCa_LumiPixelsCounts_ZeroBias_v*"),
eventSetupPathsKey='',
#HLTPaths = cms.vstring("AlCa_LumiPixelsCounts_ZeroBias_v*"),
eventSetupPathsKey='AlCa_LumiPixelsCounts_ZeroBias_v*',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
eventSetupPathsKey='AlCa_LumiPixelsCounts_ZeroBias_v*',
eventSetupPathsKey='AlCaPCCZeroBias',

@ChrisMisan
Copy link
Contributor

@Moanwar kind reminder

@ChrisMisan
Copy link
Contributor

@Moanwar ping

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 5, 2022

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39519/32443

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 5, 2022

Pull request #39519 was updated. @malbouis, @yuanchao, @cmsbuild, @saumyaphor4252, @francescobrivio, @ChrisMisan, @tvami can you please check and sign again.

@malbouis
Copy link
Contributor

malbouis commented Oct 5, 2022

test parameters:

  • workflows = 1020,1002.3,1002.4,1001.3

@malbouis
Copy link
Contributor

malbouis commented Oct 5, 2022

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 6, 2022

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9b5478/28044/summary.html
COMMIT: f5105cb
CMSSW: CMSSW_12_6_X_2022-10-05-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/39519/28044/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

There are some workflows for which there are errors in the baseline:
10224.15 step 5
11634.15 step 3
140.002 step 3
140.034 step 3
140.116 step 3
25202.15 step 5
The results for the comparisons for these workflows could be incomplete
This means most likely that the IB is having errors in the relvals.The error does NOT come from this pull request

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-9b5478/1001.3_RunSingleMuon2022B+RunSingleMuon2022B+TIER0EXPRUN3+ALCAEXPRUN3+ALCAHARVDEXPRUN3
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-9b5478/1002.3_RunZeroBias2022B+RunZeroBias2022B+TIER0PROMPTRUN3+ALCASPLITRUN3+ALCAHARVDEXPRUN3
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-9b5478/1002.4_RunDoubleMuon2022B+RunDoubleMuon2022B+TIER0PROMPTRUN3+HARVESTPROMPTRUN3
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-9b5478/1020.0_AlCaLumiPixels2021+AlCaLumiPixels2021+TIER0EXPLP+ALCAEXPLP+ALCAHARVLP+TIER0PROMPTLP
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-9b5478/41834.0_TTbar_14TeV+2026D94+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 12 differences found in the comparisons
  • DQMHistoTests: Total files compared: 53
  • DQMHistoTests: Total histograms compared: 3523122
  • DQMHistoTests: Total failures: 19961
  • DQMHistoTests: Total nulls: 7
  • DQMHistoTests: Total successes: 3503132
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.033 KiB( 52 files compared)
  • DQMHistoSizes: changed ( 138.5,... ): -0.008 KiB MessageLogger/Errors
  • DQMHistoSizes: changed ( 138.5,... ): -0.008 KiB MessageLogger/Warnings
  • DQMHistoSizes: changed ( 1002.4 ): 0.064 KiB SiStrip/MechanicalView
  • Checked 220 log files, 49 edm output root files, 53 DQM output files
  • TriggerResults: found differences in 2 / 52 workflows

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39519/32524

@cmsbuild
Copy link
Contributor

Pull request #39519 was updated. @malbouis, @yuanchao, @cmsbuild, @saumyaphor4252, @ggovi, @tvami, @ChrisMisan, @francescobrivio can you please check and sign again.

@tvami
Copy link
Contributor

tvami commented Oct 11, 2022

@cmsbuild , please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9b5478/28180/summary.html
COMMIT: ede77fe
CMSSW: CMSSW_12_6_X_2022-10-11-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39519/28180/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-9b5478/1001.3_RunSingleMuon2022B+RunSingleMuon2022B+TIER0EXPRUN3+ALCAEXPRUN3+ALCAHARVDEXPRUN3
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-9b5478/1002.3_RunZeroBias2022B+RunZeroBias2022B+TIER0PROMPTRUN3+ALCASPLITRUN3+ALCAHARVDEXPRUN3
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-9b5478/1002.4_RunDoubleMuon2022B+RunDoubleMuon2022B+TIER0PROMPTRUN3+HARVESTPROMPTRUN3
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-9b5478/1020.0_AlCaLumiPixels2021+AlCaLumiPixels2021+TIER0EXPLP+ALCAEXPLP+ALCAHARVLP+TIER0PROMPTLP
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-9b5478/41834.0_TTbar_14TeV+2026D94+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 19 differences found in the comparisons
  • DQMHistoTests: Total files compared: 53
  • DQMHistoTests: Total histograms compared: 3524328
  • DQMHistoTests: Total failures: 19964
  • DQMHistoTests: Total nulls: 7
  • DQMHistoTests: Total successes: 3504335
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.033 KiB( 52 files compared)
  • DQMHistoSizes: changed ( 138.5,... ): -0.008 KiB MessageLogger/Errors
  • DQMHistoSizes: changed ( 138.5,... ): -0.008 KiB MessageLogger/Warnings
  • DQMHistoSizes: changed ( 1002.4 ): 0.064 KiB SiStrip/MechanicalView
  • Checked 220 log files, 49 edm output root files, 53 DQM output files
  • TriggerResults: found differences in 2 / 52 workflows

@tvami
Copy link
Contributor

tvami commented Oct 12, 2022

+1

  • only change is in the types

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

@perrotta
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 77f18db into cms-sw:master Oct 12, 2022
@malbouis
Copy link
Contributor

@perrotta , @rappoccio , I would like to report something weird that seems to be happening with this PR.
In the list of files changed by this PR, there is a file completely unrelated to it: https://github.com/cms-sw/cmssw/blob/master/CondFormats/JetMETObjects/src/SimpleJetCorrector.cc
The changes in this particular file indicated as being from this PR don't seem to be effective. They are the same as implemented originally in PR #39471 and they do not appear in the github history of the file.
I thought it was worth reporting it to make extra sure no harm was done.

@perrotta
Copy link
Contributor

@malbouis thank you for telling
Chenges in that file are coming from PR #39471, which was merged right after this PR was first submitted. It doesn't look like this PR has modified anything that was coded in #39471.
I don't know how did it happen, I can only suspect that having based this PR to the wrong branch at the beginning could have brought something extra with it once automatically rebased to the master.
In any case, I don't see issue with that file or whatever else got touched by this PR.

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.

None yet

7 participants