Added TkMaps switch whether the map is TH2 or TProfile2D

[pjurgielewicz]

• tweaked version of https://github.com/cms-sw/cmssw/pull/18088/files to accomodate it to the current code
• removed direct booking via dqmStore
• overall package simplification

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22134/3260

[cmsbuild]

A new Pull Request was created by @pjurgielewicz (Pawel Jurgielewicz) for master.

It involves the following packages:

CalibTracker/SiStripQuality
DQM/SiStripCommon
DQM/SiStripMonitorClient
DQM/SiStripMonitorTrack

@ghellwig, @vazzolini, @kmaeshima, @arunhep, @cerminar, @dmitrijus, @cmsbuild, @franzoni, @jfernan2, @vanbesien, @lpernie can you please review it and eventually sign? Thanks.
@ghellwig, @erikbutz, @hdelanno, @gbenelli, @tocheng, @jlagram, @fioriNTU, @idebruyn, @mmusich, @threus, @venturia this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

[jfernan2]

please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/25928/console Started: 2018/02/06 18:21

[cmsbuild]

-1

Tested at: 3beb5b2

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-22134/25928/summary.html

I found follow errors while testing this PR

Failed tests: Build

• Build:

I found an error when building:

In file included from /cvmfs/cms-ib.cern.ch/nweek-02510/slc6_amd64_gcc630/external/gcc/6.3.0-cms/include/c++/6.3.0/memory:81:0,
                 from /cvmfs/cms-ib.cern.ch/nweek-02510/slc6_amd64_gcc630/cms/cmssw-patch/CMSSW_10_1_X_2018-02-06-1100/src/FWCore/MessageLogger/interface/MessageLogger.h:128,
                 from /build/cmsbld/jenkins/workspace/ib-any-integration/CMSSW_10_1_X_2018-02-06-1100/src/DQM/SiStripMonitorCluster/src/SiStripMonitorCluster.cc:14:
/cvmfs/cms-ib.cern.ch/nweek-02510/slc6_amd64_gcc630/external/gcc/6.3.0-cms/include/c++/6.3.0/bits/unique_ptr.h: In instantiation of 'typename std::_MakeUniq<_Tp>::__single_object std::make_unique(_Args&& ...) [with _Tp = TkHistoMap; _Args = {const TkDetMap*&, DQMStore::IBooker&, std::basic_string, std::allocator >&, const char (&)[23], double, bool}; typename std::_MakeUniq<_Tp>::__single_object = std::unique_ptr]':
/build/cmsbld/jenkins/workspace/ib-any-integration/CMSSW_10_1_X_2018-02-06-1100/src/DQM/SiStripMonitorCluster/src/SiStripMonitorCluster.cc:355:76:   required from here
/cvmfs/cms-ib.cern.ch/nweek-02510/slc6_amd64_gcc630/external/gcc/6.3.0-cms/include/c++/6.3.0/bits/unique_ptr.h:791:30: error: no matching function for call to 'TkHistoMap::TkHistoMap(const TkDetMap*&, DQMStore::IBooker&, std::basic_string&, const char [23], double, bool)'
     { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /build/cmsbld/jenkins/workspace/ib-any-integration/CMSSW_10_1_X_2018-02-06-1100/src/DQM/SiStripMonitorCluster/interface/SiStripMonitorCluster.h:14:0,
                 from /build/cmsbld/jenkins/workspace/ib-any-integration/CMSSW_10_1_X_2018-02-06-1100/src/DQM/SiStripMonitorCluster/src/SiStripMonitorCluster.cc:27:
/build/cmsbld/jenkins/workspace/ib-any-integration/CMSSW_10_1_X_2018-02-06-1100/src/DQM/SiStripCommon/interface/TkHistoMap.h:20:3: note: candidate: TkHistoMap::TkHistoMap(const TkDetMap*)


[cmsbuild]

Comparison not run due to Build errors (RelVals and Igprof tests were also skipped)

[pjurgielewicz]

Is there any way to easily obtain the full list of modules/plugins using TkHistoMap? The interface has been changed so other packages need to know about it. Thanks for help.

[mmusich]

Hi @pjurgielewicz, what about http://cmslxr.fnal.gov/ident?_i=TkHistoMap&_remember=1?

[lpernie]

+1

[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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

[fabiocos]

please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/26158/console Started: 2018/02/19 10:11

[mmusich]

@pjurgielewicz @dmitrijus @lpernie
maybe stating what is already noted, but as the type of SiStripQualityStatistics is being changed here from edm::EDAnalyzer to DQMEDAnalyzer, if this is PR is merged first, it should be taken into account later in #22244 as well.
There are several places where an update of the python configuration would be needed link to gitHub search.

[schneiml]

@mmusich for now #22244 should not cause trouble. Even if the config changes in #22243 are slightly incorrect now, this can be fixed one we actually switch to EDProducer. So I don't see a problem here atm.

However, once we move forward, these things need to be properly tested. And if these configs are not covered by the github/runTheMatrix tests, be prepared that we might break them...

[cmsbuild]

+1
Tested at: 41b4d19
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-22134/26158/summary.html

[cmsbuild]

Comparison job queued.

[mmusich]

@schneiml thanks for your reply

for now #22244 should not cause trouble. Even if the config changes in #22243 are slightly incorrect now, this can be fixed one we actually switch to EDProducer.

For my own education, as I have not followed the details of those two PRs, does it mean that even if now the module is declared to be an DQMEDAnalyzer the configuration

process.stat = cms.EDAnalyzer("SiStripQualityStatistic")

will still work (after #22243 and #22244 will be merged) without any change?

However, once we move forward, these things need to be properly tested. And if these configs are not covered by the github/runTheMatrix tests, be prepared that we might break them...

I agree something will have to be done to protect at least the critical ones. @suchandradutta @boudoul please take note.

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-22134/26158/summary.html

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 0 differences found in the comparisons
• DQMHistoTests: Total files compared: 27
• DQMHistoTests: Total histograms compared: 2465360
• DQMHistoTests: Total failures: 1
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2465190
• DQMHistoTests: Total skipped: 169
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.689999999806 KiB( 22 files compared)
• Checked 111 log files, 9 edm output root files, 27 DQM output files

[schneiml]

@mmusich currently, DQMEDAnalyzer is the same thing as cms.EDAnalyzer, and it will stay that way even with #22243 and #22244 (this was not clear in the beginning, but now it is).

So, functionally both will work and it does not matter which one you use. However, we want to use DQMEDAnalyzer in the config for all modules (and only the modules) derived from DQMEDAnalyzer, in case we want to change the base class. This will happen at some point, and then things that are incorrect will start to crash, but there is no PR for it right now.

[fabiocos]

+1