HLT DQM Cleaning #38470
HLT DQM Cleaning #38470
Conversation
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38470/30683
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38470/30684
|
|
A new Pull Request was created by @andrea21z for master. It involves the following packages:
@emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @micsucmed, @rvenditti can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ced8c1/25712/summary.html Comparison SummarySummary:
|
| if (targetMuons.at(0).isTrackerMuon()) | ||
| track0 = &*targetMuons.at(0).innerTrack(); | ||
| else if (targetMuons.at(0).isTrackerMuon()) |
There was a problem hiding this comment.
This looks like the same condition twice. ?
cmsbuild
added
code-checks-pending
and removed
tests-approved
code-checks-approved
labels
| delete[] edgesX; | ||
| if (edgesY) | ||
| delete[] edgesY; | ||
| fillEdges(nBinsY, edgesY, binParams_[binningTypeY], bookhist); |
There was a problem hiding this comment.
Now this bookhist overwrites the one computed at L462
My suggestion would be to transform fillEdges into a method that returns a bool. This could be implemented following the other suggestions of mine in this comment
There was a problem hiding this comment.
I have implemented all your comments, thank you
|
|
||
| } // End analyze() method. | ||
|
|
||
| // Method to fill binning parameters from a vector of doubles. | ||
| void HLTMuonMatchAndPlot::fillEdges(size_t& nBins, float*& edges, const vector<double>& binning) { | ||
| void HLTMuonMatchAndPlot::fillEdges(size_t& nBins, float*& edges, const vector<double>& binning, bool& bookhist) { |
There was a problem hiding this comment.
| void HLTMuonMatchAndPlot::fillEdges(size_t& nBins, float*& edges, const vector<double>& binning, bool& bookhist) { | |
| bool HLTMuonMatchAndPlot::fillEdges(size_t& nBins, float*& edges, const vector<double>& binning) { |
| if (binning.size() < 3) { | ||
| LogWarning("HLTMuonVal") << "Invalid binning parameters!"; | ||
| return; | ||
| bookhist = false; |
There was a problem hiding this comment.
| bookhist = false; | |
| return false; |
| @@ -297,6 +295,7 @@ void HLTMuonMatchAndPlot::fillEdges(size_t& nBins, float*& edges, const vector<d | |||
| const double binwidth = (binning[2] - binning[1]) / nBins; | |||
| for (size_t i = 0; i <= nBins; i++) | |||
| edges[i] = min + (binwidth * i); | |||
| bookhist = true; | |||
There was a problem hiding this comment.
| bookhist = true; |
| @@ -305,6 +304,7 @@ void HLTMuonMatchAndPlot::fillEdges(size_t& nBins, float*& edges, const vector<d | |||
| edges = new float[nBins + 1]; | |||
| for (size_t i = 0; i <= nBins; i++) | |||
| edges[i] = binning[i]; | |||
| bookhist = true; | |||
There was a problem hiding this comment.
| bookhist = true; |
| @@ -454,20 +457,22 @@ void HLTMuonMatchAndPlot::book2D(DQMStore::IBooker& iBooker, | |||
| * case. */ | |||
|
|
|||
| size_t nBinsX; | |||
| bool bookhist; | |||
There was a problem hiding this comment.
| bool bookhist; |
| float* edgesX = nullptr; | ||
| fillEdges(nBinsX, edgesX, binParams_[binningTypeX]); | ||
| fillEdges(nBinsX, edgesX, binParams_[binningTypeX], bookhist); |
There was a problem hiding this comment.
| fillEdges(nBinsX, edgesX, binParams_[binningTypeX], bookhist); | |
| bool bookhist = fillEdges(nBinsX, edgesX, binParams_[binningTypeX]); |
| delete[] edgesX; | ||
| if (edgesY) | ||
| delete[] edgesY; | ||
| fillEdges(nBinsY, edgesY, binParams_[binningTypeY], bookhist); |
There was a problem hiding this comment.
| fillEdges(nBinsY, edgesY, binParams_[binningTypeY], bookhist); | |
| bookhist &= fillEdges(nBinsY, edgesY, binParams_[binningTypeY]); |
| if (hists_[name]->getTH2F()->GetSumw2N()) | ||
| hists_[name]->enableSumw2(); | ||
|
|
||
| if (edgesX) |
There was a problem hiding this comment.
move outside of the if block
| @@ -73,7 +73,7 @@ class HLTMuonMatchAndPlot { | |||
| void endRun(const edm::Run &, const edm::EventSetup &); | |||
|
|
|||
| // Helper Methods | |||
| void fillEdges(size_t &nBins, float *&edges, const std::vector<double> &binning); | |||
| void fillEdges(size_t &nBins, float *&edges, const std::vector<double> &binning, bool &bookhist); | |||
There was a problem hiding this comment.
| void fillEdges(size_t &nBins, float *&edges, const std::vector<double> &binning, bool &bookhist); | |
| bool fillEdges(size_t &nBins, float *&edges, const std::vector<double> &binning); |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38470/30987
|
|
Pull request #38470 was updated. @emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @micsucmed, @rvenditti can you please check and sign again. |
|
please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ced8c1/26142/summary.html Comparison SummarySummary:
|
|
+1 |
cmsbuild
added
dqm-approved
fully-signed
and removed
dqm-pending
pending-signatures
labels
|
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. @perrotta, @dpiparo, @qliphy, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
This PR is created to remove unnecessary histograms for DQM HLT Muon validation and adding other interesting. Summary:
- This is an update of the PR #37117.
- Keeping only the most representative HLT folders.
- Removing histograms that are not used in the validation and adding the number of primary vertex distribution, interesting for Run 3.
PR validation: