particleFlowDisplacedVertexCandidate_cff.py imports MessageLogger_cfi

[Dr15Jones]

The configuration RecoParticleFlow/PFTracking/python/particleFlowDisplacedVertexCandidate_cff.py imports the FWCore.MessageLogger.MessageLogger_cfi and then modifies one of its parameters. The unexpected loading of the MessageLogger service by a low level _cff file is overriding the explicit setting of the Service done earlier in the files

DQM/TrackingMonitor/test/TrackingMonitor_AllTrackingSequences_cfg.py
DQM/TrackingMonitor/test/TrackingMonitor_SeedMonitor_cfg.py
DQM/TrackingMonitor/test/TrackingMonitor_StandAlone_cfg.py
DQMOffline/CalibTracker/test/SiStripBadComponentsDQMServiceReader_cfg.py
DQMOffline/CalibCalo/test/HOAlCaRecoMonStream_cfg.py
FastSimulation/Validation/test/MultiTrackValidator_cfg.py
L1Trigger/L1TCommon/test/runStandardSequences.py
RecoLocalTracker/SiStripClusterizer/test/shotTest_withReClustering_cfg.py
RecoLocalTracker/SiStripClusterizer/test/testClusterToDigi_cfg.py
Validation/RecoTrack/test/MultiTrackValidator_cfg.py

as particleFlowDisplacedVertexCandidate_cff.py is indirectly loaded by those files after they had setup the Service.

[Dr15Jones]

assign reconstruction, dqm, l1, simulation

[cmsbuild]

New categories assigned: dqm,simulation,reconstruction,l1

@mdhildreth,@jfernan2,@slava77,@andrius-k,@fioriNTU,@rekovic,@perrotta,@jmduarte,@jpata,@kmaeshima,@ErnestaP,@civanch you have been requested to review this Pull request/Issue and eventually sign? Thanks

[cmsbuild]

A new Issue was created by @Dr15Jones Chris Jones.

@Dr15Jones, @dpiparo, @silviodonato, @smuzaffar, @makortel, @qliphy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

[Dr15Jones]

This was discovered when I was reviewing changes to the MessageLogger configurations caused by a conversion script I'm using to move to the new MessageLogger syntax.

[jfernan2]

@Dr15Jones what would be the recommendation in this case: to modify particleFlowDisplacedVertexCandidate_cff.py or all the other files?

[Dr15Jones]

I would think editting particleFlowDisplacedVertexCandidate_cff.py would cause the least amount of surprise for the future. That change would require finding out why the MessageLogger is being configured there and if the reason could be solved by doing something else.

[jfernan2]

OK, adding @hatakeyamak and @bendavid into the loop then

[hatakeyamak]

We are talking about:
https://cmssdt.cern.ch/lxr/source/RecoParticleFlow/PFTracking/python/particleFlowDisplacedVertexCandidate_cff.py
Just to make sure I understand:
process.MessageLogger.suppressWarning being extended has undesired consequence?

I think we can remove this suppression with no or minimal change to particleFlowDisplacedVertexCandidate. Will try to do this cleanup.

[slava77]

after somewhat cursory check I think that the line with MessageLogger.suppressWarning can be removed from https://cmssdt.cern.ch/lxr/source/RecoParticleFlow/PFTracking/python/particleFlowDisplacedVertexCandidate_cff.py

I did not find Log[EW] in PFDisplacedVertexCandidateFinder or PFDisplacedVertexCandidateProducer.
I may have missed some intermediate or lower level tools that could issue the warnings.

[hatakeyamak]

RecoParticleFlow/PFTracking/src/PFTrackTransformer.cc may come in with LogWarning, but it looks to me LogWarning is already suppressed by another bool msgwarning. Can we just go ahead and remove MessageLogger.suppressWarning from https://cmssdt.cern.ch/lxr/source/RecoParticleFlow/PFTracking/python/particleFlowDisplacedVertexCandidate_cff.py, and confirm that we don't create unnecessary LogWarning via jenkins tests?

[slava77]

RecoParticleFlow/PFTracking/src/PFTrackTransformer.cc may come in with LogWarning, but it looks to me LogWarning is already suppressed by another bool msgwarning.

Thank you for checking deeper.

Can we just go ahead and remove MessageLogger.suppressWarning from https://cmssdt.cern.ch/lxr/source/RecoParticleFlow/PFTracking/python/particleFlowDisplacedVertexCandidate_cff.py, and confirm that we don't create unnecessary LogWarning via jenkins tests?

@hatakeyamak
may I ask you to check/consider a possible more likely sample where the warnings can appear and run on some O(100) or more events to check that this module warnings are not appearing.
Please let me know if you can make this check.
Thank you.

[hatakeyamak]

Will do today.

[Dr15Jones]

We are talking about:
https://cmssdt.cern.ch/lxr/source/RecoParticleFlow/PFTracking/python/particleFlowDisplacedVertexCandidate_cff.py
Just to make sure I understand:
process.MessageLogger.suppressWarning being extended has undesired consequence?

It is the call process.load('FWCore.MessageService.MessageLogger_cfi') that has unexpected consequences. There are top level configuration files, i.e. _cfg.py files, that explicitly create their own MessageLogger Service (i.e. not starting from FWCore.MessageService.MessageLogger_cfi) which first create the Service and then indirectly load particleFlowDisplacedVertexCandidate_cff.py. That load then overwrites the explicit Service construction with the one from FWCore.MessageService.MessageLogger_cfi which is not the intended behavior.

In general, since Services are a process wide tool, it is not a good idea to load them in _cfi or _cff files and instead best to only do it in _cfg files.

[slava77]

@Dr15Jones
what could be the mechanism to add/extend suppressWarning (or errors)?
It looks like the solutions are

• to figure out in the module implementation (and all of its called methods) how not to issue a warning/error
• to go in and edit the central MessageLogger_cfi

The former is not always practical, the latter sounds like a maintenance problem

[makortel]

RandomNumberGeneratorService has a similar issue, that is a central Service for which module-specific settings are needed. There the adopted solution is

• to go in and edit the central MessageLogger_cfi

If we follow with similar route, I'd prefer the configuration-to-be-edited to outside of FWCore (framework should not know about any specific categories or modules in user code). Maybe in Configuration.StandardSequences? Should we have separate files for different applications (like SIM, DIGI, RECO)? AFAIK HLT already has their own customizations.

[hatakeyamak]

Thanks for clarification and discussions.
For now, we just cleaned the cff file under discussion, so we are ok for this particular case.

[slava77]

+1

resolved in #32265

[jfernan2]

+1