Move MessageLogger default configuration to be part of Process #35298

makortel · 2021-09-15T21:03:24Z

PR description:

MessageLogger is one of the Services always available in the framework. Following the improved configuration validation already in python, this PR suggests to move the MessageLogger default configuration to be available in the cms.Process without any extra action. This change allows using ProcessModifiers in any cff/cfi fragment to e.g. add new categories for which Info messages would be printed out (see e.g. discussion in #35117).

Resolves cms-sw/framework-team#272

PR validation:

Framework unit tests run.

makortel · 2021-09-15T21:04:07Z

@Dr15Jones What do you think?

makortel · 2021-09-15T21:05:32Z

FWCore/ParameterSet/python/Config.py

@@ -137,6 +138,7 @@ def __init__(self,name,*Mods):
        self.options = Process.defaultOptions_()
        self.maxEvents = Process.defaultMaxEvents_()
        self.maxLuminosityBlocks = Process.defaultMaxLuminosityBlocks_()
+        self.MessageLogger = MessageLogger # intentionally not cloned


Not cloning here means that in the following

process = cms.Process("Foo") process.MessageLogger.SomeCategory = cms.untracked.PSet() process.load('FWCore.MessageService.MessageLogger_cfi')

the process.load() does not override the settings done before it.

Which, I'm assuming, is the desired behavior (to allow independent modifications of the MessageLogger in different packages).

cmsbuild · 2021-09-15T21:10:12Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35298/25308

This PR adds an extra 44KB to repository

cmsbuild · 2021-09-15T21:13:53Z

A new Pull Request was created by @makortel (Matti Kortelainen) for master.

It involves the following packages:

FWCore/MessageService (core)
FWCore/ParameterSet (core)
FWCore/PythonParameterSet (core)

@makortel, @smuzaffar, @cmsbuild, @Dr15Jones can you please review it and eventually sign? Thanks.
@wddgit 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

makortel · 2021-09-15T21:14:16Z

@cmsbuild, please test

cmsbuild · 2021-09-16T03:09:36Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ad798a/18644/summary.html
COMMIT: fadc7d0
CMSSW: CMSSW_12_1_X_2021-09-14-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35298/18644/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ad798a/18644/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ad798a/18644/git-merge-result

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 2 differences found in the comparisons
DQMHistoTests: Total files compared: 39
DQMHistoTests: Total histograms compared: 3000833
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3000805
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 38 files compared)
Checked 165 log files, 37 edm output root files, 39 DQM output files
TriggerResults: no differences found

FWCore/MessageService/python/MessageLogger_cfi.py

FWCore/ParameterSet/python/Config.py

Dr15Jones · 2021-09-16T14:02:20Z

I'm done with my initial review.

cmsbuild · 2021-09-19T23:59:41Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ad798a/18740/summary.html
COMMIT: fc16ab2
CMSSW: CMSSW_12_1_X_2021-09-19-0000/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35298/18740/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: 3 differences found in the comparisons
DQMHistoTests: Total files compared: 40
DQMHistoTests: Total histograms compared: 3211080
DQMHistoTests: Total failures: 5
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3211052
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.004 KiB( 39 files compared)
DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
Checked 169 log files, 37 edm output root files, 40 DQM output files
TriggerResults: no differences found

perrotta · 2021-09-20T06:10:13Z

@makortel now #35117 is merged. Perhaps, when the corresponding IB will become available you could take into account the comment of @fwyzard already here

cmsbuild · 2021-09-20T18:48:04Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35298/25403

This PR adds an extra 40KB to repository

cmsbuild · 2021-09-20T18:48:33Z

Pull request #35298 was updated. @makortel, @smuzaffar, @Dr15Jones can you please check and sign again.

makortel · 2021-09-20T18:53:48Z

@cmsbuild, please test

makortel · 2021-09-20T18:55:42Z

@perrotta

now #35117 is merged. Perhaps, when the corresponding IB will become available you could take into account the comment already here

I'd prefer to do cleanup in separate PRs, starting from ConfigBuilder (git grep "FWCore\.Message.*\.MessageLogger" | wc -l gives 2113).

cmsbuild · 2021-09-21T00:22:04Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ad798a/18771/summary.html
COMMIT: deafbcc
CMSSW: CMSSW_12_1_X_2021-09-20-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35298/18771/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

The workflows 140.53 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

Summary:

No significant changes to the logs found
Reco comparison results: 1299 differences found in the comparisons
DQMHistoTests: Total files compared: 40
DQMHistoTests: Total histograms compared: 3211080
DQMHistoTests: Total failures: 3671
DQMHistoTests: Total nulls: 19
DQMHistoTests: Total successes: 3207368
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -45.703 KiB( 39 files compared)
DQMHistoSizes: changed ( 140.53 ): -44.531 KiB Hcal/DigiRunHarvesting
DQMHistoSizes: changed ( 140.53 ): -1.172 KiB RPC/DCSInfo
Checked 169 log files, 37 edm output root files, 40 DQM output files
TriggerResults: no differences found

makortel · 2021-09-21T00:53:25Z

+1

cmsbuild · 2021-09-21T00:53: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 (and backports should be raised in the release meeting by the corresponding L2)

qliphy · 2021-09-21T01:35:02Z

+1

makortel mentioned this pull request Sep 15, 2021

Move MessageLogger default configuration to be part of Process cms-sw/framework-team#272

Closed

cmsbuild added this to the CMSSW_12_1_X milestone Sep 15, 2021

cmsbuild added code-checks-pending core-pending orp-pending pending-signatures tests-pending labels Sep 15, 2021

makortel commented Sep 15, 2021

View reviewed changes

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 15, 2021

cmsbuild added tests-started and removed tests-pending labels Sep 15, 2021

makortel mentioned this pull request Sep 16, 2021

CUDAService verbosity #35117

Merged

cmsbuild added tests-approved and removed tests-started labels Sep 16, 2021

fwyzard reviewed Sep 16, 2021

View reviewed changes

FWCore/MessageService/python/MessageLogger_cfi.py Show resolved Hide resolved

Dr15Jones reviewed Sep 16, 2021

View reviewed changes

FWCore/ParameterSet/python/Config.py Show resolved Hide resolved

Dr15Jones reviewed Sep 16, 2021

View reviewed changes

FWCore/ParameterSet/python/Config.py Show resolved Hide resolved

Move MessageLogger default configuration to be part of Process

fc16ab2

makortel force-pushed the messageLoggerProcess branch from fadc7d0 to fc16ab2 Compare September 17, 2021 00:51

cmsbuild added code-checks-pending tests-pending and removed tests-approved code-checks-approved labels Sep 17, 2021

cmsbuild added tests-started and removed tests-pending labels Sep 19, 2021

cmsbuild added tests-approved and removed tests-started labels Sep 19, 2021

cmsbuild added code-checks-pending tests-started and removed tests-approved code-checks-approved labels Sep 20, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 20, 2021

cmsbuild added tests-approved and removed tests-started labels Sep 21, 2021

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Sep 21, 2021

cmsbuild added orp-approved and removed orp-pending labels Sep 21, 2021

cmsbuild merged commit 5292fac into cms-sw:master Sep 21, 2021

makortel deleted the messageLoggerProcess branch September 21, 2021 13:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move MessageLogger default configuration to be part of Process #35298

Move MessageLogger default configuration to be part of Process #35298

makortel commented Sep 15, 2021

makortel commented Sep 15, 2021

makortel Sep 15, 2021

Dr15Jones Sep 16, 2021

cmsbuild commented Sep 15, 2021

cmsbuild commented Sep 15, 2021

makortel commented Sep 15, 2021

cmsbuild commented Sep 16, 2021

Dr15Jones commented Sep 16, 2021

cmsbuild commented Sep 19, 2021

perrotta commented Sep 20, 2021

cmsbuild commented Sep 20, 2021

cmsbuild commented Sep 20, 2021

makortel commented Sep 20, 2021

makortel commented Sep 20, 2021

cmsbuild commented Sep 21, 2021

makortel commented Sep 21, 2021

cmsbuild commented Sep 21, 2021

qliphy commented Sep 21, 2021

Move MessageLogger default configuration to be part of Process #35298

Move MessageLogger default configuration to be part of Process #35298

Conversation

makortel commented Sep 15, 2021

PR description:

PR validation:

makortel commented Sep 15, 2021

makortel Sep 15, 2021

Choose a reason for hiding this comment

Dr15Jones Sep 16, 2021

Choose a reason for hiding this comment

cmsbuild commented Sep 15, 2021

cmsbuild commented Sep 15, 2021

makortel commented Sep 15, 2021

cmsbuild commented Sep 16, 2021

Comparison Summary

Dr15Jones commented Sep 16, 2021

cmsbuild commented Sep 19, 2021

Comparison Summary

perrotta commented Sep 20, 2021

cmsbuild commented Sep 20, 2021

cmsbuild commented Sep 20, 2021

makortel commented Sep 20, 2021

makortel commented Sep 20, 2021

cmsbuild commented Sep 21, 2021

Comparison Summary

makortel commented Sep 21, 2021

cmsbuild commented Sep 21, 2021

qliphy commented Sep 21, 2021