HF noise event filter #31328
HF noise event filter #31328
Conversation
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31328/18088
|
|
A new Pull Request was created by @lathomas for master. It involves the following packages: PhysicsTools/NanoAOD @perrotta, @gouskos, @cmsbuild, @fgolf, @slava77, @jpata, @mariadalfonso, @santocch, @peruzzim can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins.
|
|
+1 |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
| ESHandle<CaloGeometry> pG; | ||
| iSetup.get<CaloGeometryRecord>().get(pG); | ||
| const CaloGeometry* geo = pG.product(); |
There was a problem hiding this comment.
| edm::Handle<HFRecHitCollection> theHFhits; | ||
| iEvent.getByToken(hfhits_token_, theHFhits); |
There was a problem hiding this comment.
the is typically used for the data members, better not make it possibly confused with a local.
Also, the handle itself seems unnecessary, get the hit collection directly
| edm::Handle<HFRecHitCollection> theHFhits; | |
| iEvent.getByToken(hfhits_token_, theHFhits); | |
| auto const& hfHits = iEvent.get(hfhits_token_); |
| std::vector<int> noiseBits = getNoiseBits(); | ||
|
|
||
| //Loop over the HF rechits. If one of them has Et>X and fires one the noise bits, declare the event as bad | ||
| for (HFRecHitCollection::const_iterator hfhit = theHFhits->begin(); hfhit != theHFhits->end(); hfhit++) { |
There was a problem hiding this comment.
| for (HFRecHitCollection::const_iterator hfhit = theHFhits->begin(); hfhit != theHFhits->end(); hfhit++) { | |
| for (auto const& hfhit : theHFhits) { |
range loops are preferred
| for (unsigned int i = 0; i < noiseBits.size(); i++) { | ||
| if (noiseBits[i] < 0) |
There was a problem hiding this comment.
| for (unsigned int i = 0; i < noiseBits.size(); i++) { | |
| if (noiseBits[i] < 0) | |
| for (auto noiseBit : noiseBits) { | |
| if (noiseBit < 0) |
| for (unsigned int i = 0; i < noiseBits.size(); i++) { | ||
| if (noiseBits[i] < 0) | ||
| continue; | ||
| if (((hitFlags >> noiseBits[i]) & 1) > 0) { |
There was a problem hiding this comment.
| if (((hitFlags >> noiseBits[i]) & 1) > 0) { | |
| if ((hitFlags >> noiseBits[i]) & 1) { |
this is apparently enough
| for (unsigned int i = 0; i < listOfNoises_.size(); i++) { | ||
| if (listOfNoises_[i] == "HFLongShort") |
There was a problem hiding this comment.
| for (unsigned int i = 0; i < listOfNoises_.size(); i++) { | |
| if (listOfNoises_[i] == "HFLongShort") | |
| for (auto const& noise : listOfNoises_) { | |
| if (noise == "HFLongShort") |
| } | ||
|
|
||
| std::vector<int> HFNoisyHitsFilter::getNoiseBits() const { | ||
| std::vector<int> result; |
There was a problem hiding this comment.
the conversion from listOfNoises_ as a string to a vector of ints (or better yet, HcalPhase1FlagLabels)
should be done in the constructor.
to keep that int/HcalPhase1FlagLabels vector as a const, it may be useful to keep this method as a static member taking const vector<string>& as an argument, it can then be used in the constructor initializer list as noiseBits_(getNoiseBits(iConfig.getParameter<std::vector<std::string>>("listOfNoises")))
| else if (debug_) | ||
| edm::LogWarning("HFNoisyHitsFilter") | ||
| << "Couldn't find the bit index associated to this string: " << listOfNoises_[i]; |
There was a problem hiding this comment.
this looks like a case for an exception, because a wrong noise name string would be a bug in configuration
| // ------------ method fills 'descriptions' with the allowed parameters for the module ------------ | ||
| void HFNoisyHitsFilter::fillDescriptions(edm::ConfigurationDescriptions& descriptions) { | ||
| edm::ParameterSetDescription desc; | ||
| desc.add<edm::InputTag>("hfrechits", edm::InputTag("reducedHcalRecHits:hfreco")); |
There was a problem hiding this comment.
| desc.add<edm::InputTag>("hfrechits", edm::InputTag("reducedHcalRecHits:hfreco")); | |
| desc.add<edm::InputTag>("hfrechits", {"reducedHcalRecHits:hfreco"}); |
minor, but still
|
+1 |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
@mariadalfonso Yes we need both |
|
+1
|
|
+xpog |
|
+1 |
|
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) |
cmsbuild
added
analysis-approved
fully-signed
and removed
analysis-pending
pending-signatures
labels
|
+1 |
PR description:
This PR creates an EDFilter in order to reject events containing a high Et HF hit identified as noise by one of the HCAL flags.
The list of HCAL flags to consider is configurable and currently only contains flags with very small mistag rates.
This PR complements (but doesn't replace) #31271, which aims at building discriminating variables for noise that cannot be easily distinguished from physics signal.
The PR adds one boolean to MINIAOD and NANOAOD.
The motivation for such a filter and the filter performance are described in:
https://indico.cern.ch/event/944298/#sc-10-4-hf-noise-study-on-aod
(in particular slides 4-6)
PR validation:
runTheMatrix gave no error. It was explicitly checked that the new module is run during the MINIAOD step and leads to sensible results.