Convert ObjectViewCleaner from legacy producer to stream producer #17596
Conversation
|
A new Pull Request was created by @knoepfel (Kyle Knoepfel) for CMSSW_9_0_X. 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 |
|
please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
Can this PR be merged, or are there any concerns? |
|
+1 |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_9_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar |
cmsbuild
added
dqm-approved
fully-signed
and removed
dqm-pending
pending-signatures
labels
|
+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.