Migration of a bunch of validation modules to the new DQM interfaces (3)

[vanbesien]

No description provided.

[cmsbuild]

A new Pull Request was created by @vanbesien (Broen van Besien) for CMSSW_7_3_X.

Migration of a bunch of validation modules to the new DQM interfaces (3)

It involves the following packages:

DQMOffline/EGamma

@nclopezo, @danduggan, @rovere, @cmsbuild, @deguio, @ojeda can you please review it and eventually sign? Thanks.
@rociovilar 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.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

[cmsbuild]

+1
Tested at: d733b9e
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6053/443/summary.html

[cmsbuild]

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

[rovere]

Do we really need to have these histograms included? We have put in place several other tools to monitor the number of ME recursively in each folder at which depth. Can we safely drop them?

[vanbesien]

I would love to remove them, cause it's something quite non-standard (haven't seen this kind of thing in any of the other packages) and hence they gave me some headache to migrate them.
What do you think Fede?

[deguio]

I share marco's point and I would go ahead removing them. could you please take care of this?
thanks in advance,
F.

[deguio]

thanks @vanbesien for this.
why the getParameter in:
https://github.com/vanbesien/cmssw/blob/migration_of_DQM_to_thread_safe_part_02/DQMOffline/EGamma/plugins/ZToMuMuGammaAnalyzer.cc#L74

is not performed in the ctor? if there is no reason could you move it there?
thanks,
F.

[vanbesien]

Hi @deguio it's certainly possible to move all the getParamaters to the constructor. (However: This would involve not only this class, but also some others.)

The main reason I left it in the bookHistograms methods is for code readability. In principle you want to take out the data from the parameters as late as possible. I would estimate that that is one of the aims of this class.
My first guess would be that the getParameter(...) is based on a hashtable and hence O(1).
It all depends on the cost of the getParameter(...) method vs. readability vs. the time-criticality of the bookHistograms.
I have no basis to judge which one wins, so just let me know and I'll update where necessary.

[cmsbuild]

+1
Tested at: 5c915f0
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6053/755/summary.html

[cmsbuild]

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

[deguio]

+1

[cmsbuild]

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