Faster calo correction (CaloFutureShowerOverlap)
Dear experts @cmarinbe @deschamp @graven @ibelyaev @aszabels @sstahl please review this MR which is concentrated on improving performance of CaloFutureShoweroverlap and contains two changes:
- new option (
Gaudi::Property) to skip counters inCaloFutureSCorrectionandCaloFutureLCorrection(by default counters are enabled). I guess the counetrs are slow because they are thread safe; but one cannot use buffers either because the loops inCaloFutureSCorrection::correct,CaloFutureLCorrection::correctconsists of one iteration always: are they supposed to work with a range ofCaloHypo, but this was not (yet) implemented when using these tools fromCaloFutureShowerOverlap? - using precalculated values of exponents in certain places (the total number of calls to
CaloFutureCorrectionBase::myexp()is reduced by 60%)
This improves performance CaloFutureShoweroverlap by 7% and 17% for (1) and (2), respectively, as tested on the current master branch, so the total improvement for this algorithm is about 25%.
Update 11.12.2019: precalculating values of cosh, sinh brings further improvement by 2.5%.
I noticed that Moore tests which run these algorithms and attempt to validate by comparing with reference are not sensitive to removing the counters: i.e. if one removes all counters, lines like
CaloFutureShowerOverlap.PhotonSh... SUCCESS Number of counters : 2
| Counter | # | sum | mean/eff^* | rms/err^* | min | max |
| "Delta(X)" | 285226 | -26651.25 | -0.093439 | 12.651 | -23.059 | 24.135 |
| "Delta(Y)" | 285226 | 17977.09 | 0.063028 | 12.598 | -26.049 | 22.930 |do not appear in the log files, but the test passes. I attach two output files of the RecoConf.hlt2_reco_tracking_calo_baseline test, one with counters stdout.counters0 and another one without them stdout.counters1. These output files are considered 'identical enough' such that the test passes, isn't it confusing? @rmatev can you please take a look?
Update 11.12.2019 # 2: pushed another small improvement to calculate correction and derivative in one method (they share a lot of common operations), this improves CaloFutureShowerOverlap by about 4% and I believe the code is better readable if both calculations are in one place. One might need to modify CaloFutureECorrection::calcECorrection to use the new method, and then get rid of CaloFutureECorrection::calcECorrection (for the moment I adjusted only CaloFutureSCorrection::calculateSCorrections).
Update 12.12.2019: this MR breaks certain tests (such as Brunel.brunel-upgrade-baseline) because of the disabled counters, while all tests in Moore should pass (there is some feature which prevents Moore tests to fail if counters are removed from the output).
Merge request reports
Activity
- Resolved by Marian Stahl
With respect to the counters, instead of fighting the symptom, it is probably more worthwhile to fix the cause of the problem -- which is what is being done in !1798 (merged) -- @mimazure, what is the schedule for finishing that MR?
Edited by Gerhard Raven
added 1 commit
- c9465c18 - Apply suggestion to CaloFuture/CaloFutureReco/src/CaloFutureLCorrection.h
added 1 commit
- 81f01537 - precalculated parameters stored in new simple struct
added 1 commit
- f4a21ecc - calculate correction and derivative in one function call