bugfix RPCSeedGenerator wrong nullptr check

[jhgoh]

PR description:

A nullptr-check went opposite way in the RPCSeedGenerator of the RecoMuon package, discovered thanks to @perrotta.
In addition, a minor fix to printout message is done.

The standard sequences seem to be unaffected due to this potential bug, maybe because validity checks were done using the other flags in the same if-statement.
Since this PR introduces only a safeguard from potential crash (+correction to the printout debug message) and does not include any significant change. Therefore we expect no change in outcomes of the standard reconstruction with this PR.
The latest change to this module is done with the PR #30687, which was also a technical change.

PR validation:

This PR compiles without error and passes code-checks & a runTheMatrix workflow (13250 WF for a quick check)

@andresib @mileva

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33753/22702

• This PR adds an extra 16KB to repository

[cmsbuild]

A new Pull Request was created by @jhgoh (Junghwan John Goh) for master.

It involves the following packages:

RecoMuon/MuonSeedGenerator

@perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@HuguesBrun, @calderona, @abbiendi, @Fedespring, @bellan, @sscruz, @trocino, @cericeci, @rociovilar this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

[perrotta]

type bugfix

[perrotta]

please test

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-99ecee/15138/summary.html
COMMIT: a521c64
CMSSW: CMSSW_12_0_X_2021-05-16-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33753/15138/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: 0 differences found in the comparisons
• DQMHistoTests: Total files compared: 37
• DQMHistoTests: Total histograms compared: 2648242
• DQMHistoTests: Total failures: 1
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2648219
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
• Checked 155 log files, 37 edm output root files, 37 DQM output files
• TriggerResults: no differences found

[perrotta]

Thank you @jhgoh

In fact, this (legacy!) EDProducer doesn't even seem to be used anywhere: a rpcMuonSeed module is instanciated in DQM/RPCMonitorDigi/python/RPCMuonSeeds_cfi.py, also linked from DQM/RPCMonitorDigi/python/RPCSeedGenerator_cff.py, but then it is not used anywhere in CMSSW. It perhaps enters some private DQM sequence used by experts (as John suggests).

However, the fix is rather straightforward, and it is worth to fix it.

[perrotta]

+1

• Simple and logical fixes on a producer probably never used anywhere in CMSSW
• Jenkins tests pass

[cmsbuild]

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

[qliphy]

@jhgoh cout is not thread-safe, would you please take the chance to replace it with LogVerbatim or LogInfo or something similar (see https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideMessageLogger )?

[perrotta]

@jhgoh cout is not thread-safe, would you please take the chance to replace it with LogVerbatim or LogInfo or something similar (see https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideMessageLogger )?

@qliphy that's correct, but this code is never used in production (it is also a legacy EDProducer, and that also should have to be fixed at some point).
Besides, the trivial fixes applied here, I don't think it is worth spending time and efforts in trying to maintain it: if any, I'd rather investigate with @cms-sw/rpc-dpg-l2 whether it is really needed, and if it is found to be obsolete I'd rather fully remove it from the release. Alternatively, if only used in some private DQM test, it could be moved into the \test area of the package, instead.

[qliphy]

+1

[perrotta]

@jhgoh @qliphy I've opened issue #33761 to keep track of the RPC code that needs to be either cleaned up or updated, as the RPCSeedGenerator encountered here