Add rhoCorr and energy-binned cuts #18559
Conversation
|
A new Pull Request was created by @afiqaize (Afiq Anuar) for master. It involves the following packages: HLTrigger/Egamma @Martin-Grunewald, @silviodonato, @cmsbuild, @fwyzard, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
This looks the wrong strategy: |
|
The idea is to have these capabilities with as little migration work as possible, given the timescale we have - there are just many HLTEgammaGenericFilters in the menu. I'm not sure I understand the last part about the producer - currently all the relevant producers also have the same bit of duplicate code - so this is just a matter of shifting where this correction is done (without creating another collection, not sure if this would be cheaper). What do you think is a more efficient migration strategy? |
|
Pull request #18559 was updated. @Martin-Grunewald, @silviodonato, @cmsbuild, @fwyzard, @davidlange6 can you please check and sign again. |
|
Code duplication is never a good idea. |
|
For the upcoming single ele retuning we foresee 2 working points; Loose and Tight. Let me try to clarify the motivation behind this PR. Currently, we have 2 seeded H/E producers in the menu: with and without rhoCorr. The latter one serves only the single electron paths. As to apply rhoCorr is part of a WP, with this PR we can have only single ele WPs apply the rhoCorr on H/E and others don't, without branching off on the producer, which we think is the more consistent approach (and less chance for definition-affecting desync). Similarly, last year with our effArea updates we more or less "forced" other WPs to adopt them, due to not wanting to branch off the isolation producers. If this PR is accepted, the plan is to: |
| double cutOverE2EB2_ = 9999., cutOverE2EE2_ = 9999.; | ||
|
|
||
| int iEn = -1; | ||
| for (int dIt = energyLowEdges_.size() - 1; dIt > -1; dIt--) { |
There was a problem hiding this comment.
please consider replacing this loop with something like std::lower_bound or std::upper_bound(energyLowEdges_.begin(), energyLowEdges_.end(), energy)
|
|
||
| int iEn = -1; | ||
| for (int dIt = energyLowEdges_.size() - 1; dIt > -1; dIt--) { | ||
| if ( energy >= energyLowEdges_.at(dIt) ) { |
There was a problem hiding this comment.
please consider replacing this loop with something like std::lower_bound or std::upper_bound(energyLowEdges_.begin(), energyLowEdges_.end(), energy)
| double cutOverE2EB_ = 9999., cutOverE2EE_ = 9999.; | ||
|
|
||
| int iEn = -1; | ||
| for (int dIt = energyLowEdges_.size() - 1; dIt > -1; dIt--) { |
There was a problem hiding this comment.
please consider replacing this loop with something like std::lower_bound or std::upper_bound(energyLowEdges_.begin(), energyLowEdges_.end(), energy)
There was a problem hiding this comment.
Right, thanks for pointing it out.
|
Pull request #18559 was updated. @Martin-Grunewald, @silviodonato, @cmsbuild, @fwyzard, @davidlange6 can you please check and sign again. |
|
This PR has now become a 92X PR - please make a PR for 91X as well. |
|
please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
cmsbuild
added
tests-approved
tests-pending
and removed
tests-started
tests-approved
labels
|
Pull request #18559 was updated. @Martin-Grunewald, @silviodonato, @cmsbuild, @fwyzard, @davidlange6 can you please check and sign again. |
cmsbuild
added
comparison-pending
hlt-pending
pending-signatures
tests-pending
and removed
tests-approved
labels
|
Pull request #18559 was updated. @Martin-Grunewald, @silviodonato, @cmsbuild, @fwyzard, @davidlange6 can you please check and sign again. |
|
Pull request #18559 was updated. @Martin-Grunewald, @silviodonato, @cmsbuild, @fwyzard, @davidlange6 can you please check and sign again. |
|
please test |
|
The tests are being triggered in jenkins. |
|
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
|
Comparison job queued. |
|
+1 |
cmsbuild
added
fully-signed
hlt-approved
and removed
hlt-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 requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar |
|
Comparison is ready Comparison Summary:
|
|
+1 |
[Take 2 of the closed #18558]
This PR adds 2 new features to the affected filters:
1 - ability to do rho correction on the variable being cut on, so that this can be applied at filter level instead of producer level as is done now
2 - energy or Et dependent cut thresholds for e.g. looser dPhi cut at low Et
The customization function provided modifies the affected filters such that they're compatible with this PR but with no change in behavior at all.
Cheers,
Afiq