SiPixelFolderOrganizer update in 72X

[leggat]

72X version of #4425

[cmsbuild]

A new Pull Request was created by @leggat for CMSSW_7_2_X.

SiPixelFolderOrganizer update in 72X

It involves the following packages:

DQM/SiPixelCommon
DQM/SiPixelMonitorCluster
DQM/SiPixelMonitorDigi
DQM/SiPixelMonitorRawData
DQM/SiPixelMonitorRecHit
DQM/SiPixelMonitorTrack

@ojeda, @danduggan, @rovere, @cmsbuild, @nclopezo, @deguio, @Degano can you please review it and eventually sign? Thanks.
@threus this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.

[cmsbuild]

+1
Tested at: 7085e4a
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4507/254/summary.html

[cmsbuild]

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

[deguio]

hello @leggat
few changes are still needed. the direct instantiation of the DQMStore is forbidden. I am referring to:
https://github.com/leggat/cmssw/blob/folderOrganizer72X/DQM/SiPixelCommon/src/SiPixelFolderOrganizer.cc#L32

and similar lines. could you change the SiPixelFolderOrganizer to use the ibooker instead?
please let me know if you have questions.
thanks in advance,
F.

[deguio]

actually what I am asking is probably to completely remove the methods which use the DQMStore directly. I see that you have implemented them to use the ibooker with a different signature.

[leggat]

Hi @deguio ,
I originally tried to make the changes like that in leggat@a4146ff from #4425 , but this caused conflicts in various codes in CalibTracker/SiPixelTools that relied on the original functionality of the SiPixelFolderOrganizer. That was why I put in the overloaded functions with different signatures - one in which the DQM framework wouldn't access the DQMStore at all but then the other so that these other codes would still run.
Is it now completely forbidden to access/instantiate the DQMStore from ALL code then, even on these CalibTracker codes running locally? Because if so then those codes will also need changing, and I'm not sure how they work or what they do, just that they try to call this function...
Cheers,
Duncan

[deguio]

hello @leggat
it is forbidden in the sense that soon it will be possible to book histograms only through the iBooker. the rest will be made private in the DQMStore class. so yes, also the private code should be migrated.
I will try to figure out who is the responsible for that code while keeping this PR on hold.
thanks for the explanation.
cheers,
F.

[deguio]

of course, since it is purely a technical change, feel free to migrate the modules in CalibTracker/SiPixelTools if you feel like.. ;)

[deguio]

+1
approving not to loose these changes. once the following packages will be migrated, it will be possible to remove the old methods.

CalibTracker/SiPixelTools/interface/SiPixelOfflineCalibAnalysisBase.h
CalibTracker/SiPixelTools/plugins/SiPixelErrorsDigisToCalibDigis.h
DQM/SiPixelMonitorClient/interface/SiPixelMuonHLT.h
DQM/SiPixelMonitorClient/src/SiPixelActionExecutor.cc
DQM/SiPixelMonitorClient/src/SiPixelDataQuality.cc
DQM/SiPixelMonitorClient/src/SiPixelInformationExtractor.cc

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_2_X IBs unless changes (tests are also fine).