Skip to content

Conversation

@Dr15Jones
Copy link
Contributor

PR description:

  • converted FWCore/MessageService/python/MessageLogger_cfi.py to new syntax
  • converted all uses of that _cfi file which modify the configuration to also use the new syntax.

PR validation:

  • Ran all modified files under python. Any failures seen were not due to this change.
  • All FWCore unit test passed

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32244/19999

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32244/20000

@santocch
Copy link

+1

@Martin-Grunewald
Copy link
Contributor

Martin-Grunewald commented Dec 17, 2020

@Dr15Jones
In TSG/HLT, we use

    process.MessageLogger.categories.append('TriggerSummaryProducerAOD')
    process.MessageLogger.categories.append('L1GtTrigReport')
    process.MessageLogger.categories.append('L1TGlobalSummary')
    process.MessageLogger.categories.append('HLTrigReport')
    process.MessageLogger.categories.append('FastReport')

but it seems categories has disappeared from the MessageLogger (in ConfDB parsing for 11_3_0_pre1):

-------------------------------------------------------------------------------
Services (1):
  -> MessageLogger [MessageLogger] CHANGED
       untracked PSet files [ADDED]
       untracked vstring statistics [REMOVED]
       untracked PSet cerr_stats [REMOVED]
       untracked vstring categories [REMOVED]
       untracked vstring destinations [REMOVED]

ie, it was removed, and append now gives an error!

What should we use as replacement to get the above messages?
Possibly process.MessageLogger.TriggerSummaryProducerAOD=dict()
but is this enough to get the printout?

@Dr15Jones
Copy link
Contributor Author

Possibly process.MessageLogger.TriggerSummaryProducerAOD=dict()
but is this enough to get the printout?

Yes, that will allow INFO messages to be seen as the default is still to have a threshold of INFO but with the default limit=0.

For reference, the how to on the new syntax can be found here
https://github.com/cms-sw/cmssw/blob/master/FWCore/MessageService/Readme.md

@Martin-Grunewald
Copy link
Contributor

Martin-Grunewald commented Dec 17, 2020

Hmm, still a problem:

----- Begin Fatal Exception 17-Dec-2020 16:51:05 CET-----------------------
An exception of category 'ConfigFileReadError' occurred while
   [0] Processing the python configuration file named OnLine_HLT_GRun.py
Exception Message:
 unknown python problem occurred.
TypeError: TriggerSummaryProducerAOD does not already exist, so it can only be \
set to a CMS python configuration type

At:
  /cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc900/cms/cmssw-patch/CMSSW_11_3_X_20\
20-12-16-2300/python/FWCore/ParameterSet/Mixins.py(299): __raiseBadSetAttr
  /cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc900/cms/cmssw-patch/CMSSW_11_3_X_20\
20-12-16-2300/python/FWCore/ParameterSet/Mixins.py(244): __addParameter
  /cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc900/cms/cmssw-patch/CMSSW_11_3_X_20\
20-12-16-2300/python/FWCore/ParameterSet/Mixins.py(276): __setattr__
  OnLine_HLT_GRun.py(92189): <module>

----- End Fatal Exception -------------------------------------------------

where the offending lines around 92189 are:

if 'MessageLogger' in process.__dict__:
    process.MessageLogger.TriggerSummaryProducerAOD=dict()
    process.MessageLogger.L1GtTrigReport=dict()
    process.MessageLogger.L1TGlobalSummary=dict()
    process.MessageLogger.HLTrigReport=dict()
    process.MessageLogger.FastReport=dict()

??
The py file does contain:

process.hltTriggerSummaryAOD = cms.EDProducer( "TriggerSummaryProducerAOD",

@Dr15Jones
Copy link
Contributor Author

@Martin-Grunewald I believe the problem is you do not start from the FWCore.MessageService.MessageLogger_cfi file for your initial setup of the MessageLogger. That cfi contains the information that constrains the allowed values of previously unknown MessageLogger parameters to be PSets and therefore one can use dict() to initialize them. In your case, you do not have that information so should do

   process.MessageLogger.TriggerSummaryProducerAOD=cms.untracked.PSet()

which explicitly set the type.

@Dr15Jones
Copy link
Contributor Author

@Martin-Grunewald out of curiousity, what do you get if you do

process.MessageLogger.dumpPython()

?

@Martin-Grunewald
Copy link
Contributor

No output...

@Dr15Jones
Copy link
Contributor Author

print(process.MessageLogger.dumpPython())

@Martin-Grunewald
Copy link
Contributor

cms.Service("MessageLogger",
    cerr = cms.untracked.PSet(
        FwkJob = cms.untracked.PSet(
            limit = cms.untracked.int32(0)
        ),
        FwkReport = cms.untracked.PSet(
            limit = cms.untracked.int32(0),
            reportEvery = cms.untracked.int32(1)
        ),
        FwkSummary = cms.untracked.PSet(
            limit = cms.untracked.int32(10000000),
            reportEvery = cms.untracked.int32(1)
        ),
        INFO = cms.untracked.PSet(
            limit = cms.untracked.int32(0)
        ),
        Root_NoDictionary = cms.untracked.PSet(
            limit = cms.untracked.int32(0)
        ),
        default = cms.untracked.PSet(
            limit = cms.untracked.int32(10000000)
        ),
        noTimeStamps = cms.untracked.bool(False),
        suppressDebug = cms.untracked.vstring(),
        suppressError = cms.untracked.vstring(),
        suppressInfo = cms.untracked.vstring(),
        suppressWarning = cms.untracked.vstring(),
        threshold = cms.untracked.string('INFO')
    ),
    cout = cms.untracked.PSet(
        placeholder = cms.untracked.bool(True)
    ),
    debugModules = cms.untracked.vstring(),
    suppressDebug = cms.untracked.vstring(),
    suppressError = cms.untracked.vstring(
        'hltOnlineBeamSpot', 
        'hltL3MuonCandidates', 
        'hltL3TkTracksFromL2OIState', 
        'hltPFJetCtfWithMaterialTracks', 
        'hltL3TkTracksFromL2IOHit', 
        'hltL3TkTracksFromL2OIHit'
    ),
    suppressFwkInfo = cms.untracked.vstring(),
    suppressInfo = cms.untracked.vstring(),
    suppressWarning = cms.untracked.vstring(
        'hltOnlineBeamSpot', 
        'hltCtf3HitL1SeededWithMaterialTracks', 
        'hltL3MuonsOIState', 
        'hltPixelTracksForHighMult', 
        'hltHITPixelTracksHE', 
        'hltHITPixelTracksHB', 
        'hltCtfL1SeededWithMaterialTracks', 
        'hltRegionalTracksForL3MuonIsolation', 
        'hltSiPixelClusters', 
        'hltActivityStartUpElectronPixelSeeds', 
        'hltLightPFTracks', 
        'hltPixelVertices3DbbPhi', 
        'hltL3MuonsIOHit', 
        'hltPixelTracks', 
        'hltSiPixelDigis', 
        'hltL3MuonsOIHit', 
        'hltL1SeededElectronGsfTracks', 
        'hltL1SeededStartUpElectronPixelSeeds', 
        'hltBLifetimeRegionalCtfWithMaterialTracksbbPhiL1FastJetFastPV', 
        'hltCtfActivityWithMaterialTracks'
    ),
    threshold = cms.untracked.string('INFO')
)

but I had to comment out the offending assignments.

@Martin-Grunewald
Copy link
Contributor

And when I put them back with process.MessageLogger.TriggerSummaryProducerAOD=cms.untracked.PSet()
then I get:

cms.Service("MessageLogger",
    FastReport = cms.untracked.PSet(

    ),
    HLTrigReport = cms.untracked.PSet(

    ),
    L1GtTrigReport = cms.untracked.PSet(

    ),
    L1TGlobalSummary = cms.untracked.PSet(

    ),
    TriggerSummaryProducerAOD = cms.untracked.PSet(

    ),
    cerr = cms.untracked.PSet(
        FwkJob = cms.untracked.PSet(
            limit = cms.untracked.int32(0)
        ),
        FwkReport = cms.untracked.PSet(
            limit = cms.untracked.int32(0),
            reportEvery = cms.untracked.int32(1)
        ),
        FwkSummary = cms.untracked.PSet(
            limit = cms.untracked.int32(10000000),
            reportEvery = cms.untracked.int32(1)
        ),
        INFO = cms.untracked.PSet(
            limit = cms.untracked.int32(0)
        ),
        Root_NoDictionary = cms.untracked.PSet(
            limit = cms.untracked.int32(0)
        ),
        default = cms.untracked.PSet(
            limit = cms.untracked.int32(10000000)
        ),
        noTimeStamps = cms.untracked.bool(False),
        suppressDebug = cms.untracked.vstring(),
        suppressError = cms.untracked.vstring(),
        suppressInfo = cms.untracked.vstring(),
        suppressWarning = cms.untracked.vstring(),
        threshold = cms.untracked.string('INFO')
    ),
    cout = cms.untracked.PSet(
        placeholder = cms.untracked.bool(True)
    ),
    debugModules = cms.untracked.vstring(),
    suppressDebug = cms.untracked.vstring(),
    suppressError = cms.untracked.vstring(
        'hltOnlineBeamSpot', 
        'hltL3MuonCandidates', 
        'hltL3TkTracksFromL2OIState', 
        'hltPFJetCtfWithMaterialTracks', 
        'hltL3TkTracksFromL2IOHit', 
        'hltL3TkTracksFromL2OIHit'
    ),
    suppressFwkInfo = cms.untracked.vstring(),
    suppressInfo = cms.untracked.vstring(),
    suppressWarning = cms.untracked.vstring(
        'hltOnlineBeamSpot', 
        'hltCtf3HitL1SeededWithMaterialTracks', 
        'hltL3MuonsOIState', 
        'hltPixelTracksForHighMult', 
        'hltHITPixelTracksHE', 
        'hltHITPixelTracksHB', 
        'hltCtfL1SeededWithMaterialTracks', 
        'hltRegionalTracksForL3MuonIsolation', 
        'hltSiPixelClusters', 
        'hltActivityStartUpElectronPixelSeeds', 
        'hltLightPFTracks', 
        'hltPixelVertices3DbbPhi', 
        'hltL3MuonsIOHit', 
        'hltPixelTracks', 
        'hltSiPixelDigis', 
        'hltL3MuonsOIHit', 
        'hltL1SeededElectronGsfTracks', 
        'hltL1SeededStartUpElectronPixelSeeds', 
        'hltBLifetimeRegionalCtfWithMaterialTracksbbPhiL1FastJetFastPV', 
        'hltCtfActivityWithMaterialTracks'
    ),
    threshold = cms.untracked.string('INFO')
)


@Dr15Jones
Copy link
Contributor Author

@Martin-Grunewald as I suspected, the various cms.optional items are not part of your base MessageLogger configuration and therefore one has to specify the exact types.

@Dr15Jones
Copy link
Contributor Author

@Martin-Grunewald do you know where

    cout = cms.untracked.PSet(
        placeholder = cms.untracked.bool(True)
    ),

comes from in your configuration? placeholder is no longer required/allowed with the new syntax.

@Martin-Grunewald
Copy link
Contributor

ConfDB keeps nested parameters (ie parameters inside PSets) and adds/removes top level parameters (only), but at the same time we can add/remove those by hand.

@Dr15Jones
Copy link
Contributor Author

The easiest would be just to remove the whole cout PSet all together as you have no need for it.

@Martin-Grunewald
Copy link
Contributor

This is a top level parameter so ConfDB foces it to exist when seeing it in

MessageLogger                                     FWCore/MessageLogger/python/MessageLogger_cfi.py

@Dr15Jones
Copy link
Contributor Author

But the settings for cout you are using do not match what is in FWCore/MessageLogger/python/MessageLogger_cfi.py (which is a redirection to FWCore/MessageService/python/MessageLogger_cfi.py.

@Dr15Jones
Copy link
Contributor Author

So I am presently running from master, and I get

[root@25fa425c5ca5 CMSSW_11_2_0_pre8]# python
Python 2.7.15+ (default, May  7 2020, 19:47:02) 
[GCC 8.4.0] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from FWCore.MessageService.MessageLogger_cfi import MessageLogger
>>> print(MessageLogger.cout.dumpPython())
cms.untracked.PSet(
    enable = cms.untracked.bool(False),
    enableStatistics = cms.untracked.bool(False),
    lineLength = cms.optional.untracked.int32,
    noLineBreaks = cms.optional.untracked.bool,
    noTimeStamps = cms.optional.untracked.bool,
    resetStatistics = cms.untracked.bool(False),
    statisticsThreshold = cms.optional.untracked.string,
    threshold = cms.optional.untracked.string,
    allowAnyLabel_=cms.optional.untracked.PSetTemplate(
        limit = cms.optional.untracked.int32,
        reportEvery = cms.untracked.int32(1),
        timespan = cms.optional.untracked.int32
    )
)
>>> 

@Martin-Grunewald
Copy link
Contributor

That's what I mean: cout is a top level parameter, so ConfDB parsing keeps it as it sees it during parsing of the MessageLogger cfi.
But parsing does NOT update the nested parameters inside a top-level PSet (because they could contain HLT specific values we want to keep).

@Martin-Grunewald
Copy link
Contributor

Hmm, for some reason ConfDB parsing keeps both cout and cerr...

@Dr15Jones
Copy link
Contributor Author

NOTE: in my dump I just dumped MessageLogger.cout, not the full MessageLogger.

@Martin-Grunewald
Copy link
Contributor

Ah, ok.

@Martin-Grunewald
Copy link
Contributor

The next error is:

----- Begin Fatal Exception 18-Dec-2020 06:31:02 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing service of type MessageLogger
Exception Message:
Illegal parameters found in configuration.  The parameters are named:
 'suppressDebug'
 'suppressError'
 'suppressInfo'
 'suppressWarning'
You could be trying to use parameter names that are not
allowed for this plugin or they could be misspelled.

 The preceding exception was thrown in MessageLoggerScribe
and forwarded to the main thread from the Messages thread.
----- End Fatal Exception -------------------------------------------------

I do not understand why these parameters which are in the cfi now cause an error!

@Martin-Grunewald
Copy link
Contributor

Martin-Grunewald commented Dec 18, 2020

Looks like the above are caused by the nested incarnations (inside cerr), so I can remove them. But then I get an error in threshold and here it seems it is the top level threshold I need to remove, but it is forced to exist by ConfDB parsing.

Please advise.

@Martin-Grunewald
Copy link
Contributor

Martin-Grunewald commented Dec 18, 2020

It seems I can get rid of threshold in the extracted py config by setting it to empty in the ConfDB GUI, rather than any value.

@makortel
Copy link
Contributor

But then I get an error in threshold and here it seems it is the top level threshold I need to remove, but it is forced to exist by ConfDB parsing.

I'm abit puzzled because there should not be any "top level threshold" in the MessageLogger_cfi.py (while many of the contained PSets, like MessageLogger.cerr do have threshold).

Written that, does #32537 mean that you succeeded in migrating the MessageLogger configuration in ConfDB to the "new syntax"?

@Martin-Grunewald
Copy link
Contributor

Yes indeed. It must be some ConfDB idiosyncrasy, maybe because it is an untracked parameter. (It may be that in the distant past this untracked parameter was added by hand/hardwired in ConfDB's template for MessageLogger, which is/was a brittle feature we no longer use)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment