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

Drop type specs for L1TMonitor and L1TMonitorClient #36588

Merged
merged 15 commits into from Jan 4, 2022

Conversation

NiharSaha
Copy link
Contributor

PR description:

Drop type specs in DQM/L1TMonitor, DQM/L1TMonitorClient python configuration files.

Central DQM migration campaign for drop type spces following
https://cms-sw.github.io/cms_coding_rules.html#6--packaging-rules-1
and update the safer syntax for existing parameters like: (1) drop type specifications where the original parameter exists. (2) using "clone" instead of "deepcopy" (3) move all parameters inside the clone

PR validation:

runTheMatrix.py -l limited -i all --ibeos

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36588/27554

  • This PR adds an extra 28KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @NiharSaha (Nihar Ranjan Saha) for master.

It involves the following packages:

  • DQM/L1TMonitor (dqm)
  • DQM/L1TMonitorClient (dqm)

@emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @pbo0, @rvenditti can you please review it and eventually sign? Thanks.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

l1TdeRCTfromRCT.HistFolder = cms.untracked.string('L1TEMU/L1TdeRCT_FromRCT')

l1TdeRCTRun1 = l1TdeRCT.clone(
rctSourceData = 'caloStage1Digis',
Copy link
Contributor

Choose a reason for hiding this comment

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

this change is not the same: l1TdeRCTRun1 should be a frs clone of l1TdeRCT and then the pvalues of l1TdeRCT should be changed, please leave as it was

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, Thanks. Updated as it was in the new commit.

l1tRctfromRCT.HistFolder = cms.untracked.string('L1T/L1TRCT_FromRCT')

l1tRctRun1 = l1tRct.clone(
rctSource = 'caloStage1Digis',
Copy link
Contributor

Choose a reason for hiding this comment

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

same as before

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36588/27558

  • This PR adds an extra 32KB to repository

@cmsbuild
Copy link
Contributor

Pull request #36588 was updated. @emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @pbo0, @rvenditti can you please check and sign again.

@jfernan2
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a70c32/21469/summary.html
COMMIT: 78fa11b
CMSSW: CMSSW_12_3_X_2021-12-24-2300/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36588/21469/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: 2 differences found in the comparisons
  • DQMHistoTests: Total files compared: 43
  • DQMHistoTests: Total histograms compared: 3459046
  • DQMHistoTests: Total failures: 237
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 3458786
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -2605.568 KiB( 42 files compared)
  • DQMHistoSizes: changed ( 10024.0,... ): -151.212 KiB L1TEMU/L1TdeStage2uGMT
  • DQMHistoSizes: changed ( 10024.0,... ): -0.225 KiB L1TEMU/L1TdeStage2uGT
  • DQMHistoSizes: changed ( 11634.0,... ): -155.661 KiB L1TEMU/L1TdeStage2uGMT
  • DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
  • Checked 181 log files, 42 edm output root files, 43 DQM output files
  • TriggerResults: no differences found

muonCollection2 = (emulatorModule, "imdMuonsBMTF")
monitorDir = ugmtEmuImdMuDqmDir+"/BMTF/data_vs_emulator_comparison"
summaryTitle = "Summary of uGMT intermediate muon from BMTF comparison between unpacked and emulated"
ignoreBin = cms.untracked.vint32(ignoreBins)
Copy link
Contributor

Choose a reason for hiding this comment

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

can this type be dropped?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it can't be dropped as it is not already defined. While I tried to remove type it was showing error.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. You can define a default value in l1tdeStage2uGMT (empty vector), so that it can be modified everywhere else; or alternatively let the type specified, as it is now (not to be preferred, though)

muonCollection2 = (emulatorModule, "imdMuonsOMTFNeg")
monitorDir = (ugmtEmuImdMuDqmDir+"/OMTF_neg/data_vs_emulator_comparison")
summaryTitle = ("Summary of uGMT intermediate muon from OMTF- comparison between unpacked and emulated")
ignoreBin = cms.untracked.vint32(ignoreBins)
Copy link
Contributor

Choose a reason for hiding this comment

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

can this type be dropped?

muonCollection2 = (emulatorModule, "imdMuonsOMTFPos")
monitorDir = (ugmtEmuImdMuDqmDir+"/OMTF_pos/data_vs_emulator_comparison")
summaryTitle = "Summary of uGMT intermediate muon from OMTF+ comparison between unpacked and emulated"
ignoreBin = cms.untracked.vint32(ignoreBins)
Copy link
Contributor

Choose a reason for hiding this comment

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

can this type be dropped?

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a70c32/21508/summary.html
COMMIT: 11a4f35
CMSSW: CMSSW_12_3_X_2021-12-31-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36588/21508/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: 2 differences found in the comparisons
  • DQMHistoTests: Total files compared: 43
  • DQMHistoTests: Total histograms compared: 3461624
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3461596
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -3.825 KiB( 42 files compared)
  • DQMHistoSizes: changed ( 10024.0,... ): -0.225 KiB L1TEMU/L1TdeStage2uGT
  • Checked 181 log files, 42 edm output root files, 43 DQM output files
  • TriggerResults: no differences found

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 4, 2022

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36588/27624

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 4, 2022

Pull request #36588 was updated. @emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @pbo0, @rvenditti can you please check and sign again.

@jfernan2
Copy link
Contributor

jfernan2 commented Jan 4, 2022

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 4, 2022

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a70c32/21523/summary.html
COMMIT: 9293a2f
CMSSW: CMSSW_12_3_X_2022-01-03-2300/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36588/21523/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: 43
  • DQMHistoTests: Total histograms compared: 3461692
  • DQMHistoTests: Total failures: 11
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 3461658
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.004 KiB( 42 files compared)
  • DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
  • Checked 181 log files, 42 edm output root files, 43 DQM output files
  • TriggerResults: no differences found

@jfernan2
Copy link
Contributor

jfernan2 commented Jan 4, 2022

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 4, 2022

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

@qliphy
Copy link
Contributor

qliphy commented Jan 4, 2022

+1

@cmsbuild cmsbuild merged commit ca33c02 into cms-sw:master Jan 4, 2022
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

5 participants