Skip to content

Faster calo correction (CaloFutureShowerOverlap)

Oleksandr Zenaiev requested to merge faster-calo-correction into master

Dear experts @cmarinbe @deschamp @graven @ibelyaev @aszabels @sstahl please review this MR which is concentrated on improving performance of CaloFutureShoweroverlap and contains two changes:

  1. new option (Gaudi::Property) to skip counters in CaloFutureSCorrection and CaloFutureLCorrection (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 in CaloFutureSCorrection::correct, CaloFutureLCorrection::correct consists of one iteration always: are they supposed to work with a range of CaloHypo, but this was not (yet) implemented when using these tools from CaloFutureShowerOverlap?
  2. 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).

Edited by Oleksandr Zenaiev

Merge request reports