Convert ObjectViewCleaner from legacy producer to stream producer #17715
Conversation
|
A new Pull Request was created by @cmsbuild for master. It involves the following packages: Validation/RecoTau @cmsbuild, @dmitrijus, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here #13028 |
|
Can this PR be retested and merged if the tests pass? Thanks. |
|
please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
+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 requires discussion in the ORP meeting before it's merged. @Muzaffar, @davidlange6, @smuzaffar |
|
+1 |
Significant changes were implemented during the conversion:
In the legacy version, a C-style array of Booleans was used in the determination of whether to consider a candidate "clean". The array was necessary because of a set of nested loops that were inverted with respect to the natural logic of the module. I removed the C-style array and placed the "candidates" loop on the outside, so that a simple
if (shouldKeep(candidate)...)statement could be used to determine if a candidate is clean.In one of the inner loops, the Boolean variable was being set to
false, indicating that the candidate should not be considered clean. However, instead of moving on to the next candidate, the loop continued until the entire container was traversed. Since thefalseflag could never be turned back totrue, this approach was inefficient. I replaced that implementation with a function call where, if the candidate overlaps with a (e.g.) jet, instead of setting a flag, the function just returnsfalse.I would appreciate it if the maintainers can check the changes in logic. I do not believe I have introduced errors, but that statement is based on just testing
runTheMatrix.py -l 9.0, which may be insufficient.Automatically ported from CMSSW_9_0_X #17596 (original by @knoepfel).
Please wait for a new IB (12 to 24H) before requesting to test this PR.