Cross Correlation algorithm for ECAL timing, first implementation #30

nminafra · 2021-02-12T15:03:20Z

This is the first attempt of integrating the Cross Correlation method in the ECAL reconstruction code.
The code was rebased on 11_3_X.

NOTE: For this version of the code the default algorithm is still the RatioMethod.
To enable the Cross Correlation method, you need to change:
timealgo = cms.string("RatioMethod") -> timealgo = cms.string("KansasCC")
in
RecoLocalCalo/EcalRecProducers/python/ecalMultiFitUncalibRecHit_cfi.py

cmssw/RecoLocalCalo/EcalRecProducers/python/ecalMultiFitUncalibRecHit_cfi.py

Line 36 in ffacaad

timealgo = cms.string("RatioMethod"),

thomreis

First pass.
Please run also
scram build code-checks
scram build code-format
already now.

RecoLocalCalo/EcalRecAlgos/interface/EcalUncalibRecHitTimingCCAlgo.h

RecoLocalCalo/EcalRecAlgos/src/EcalUncalibRecHitTimingCCAlgo.cc

RecoLocalCalo/EcalRecAlgos/interface/EcalUncalibRecHitTimingCCAlgo.h

RecoLocalCalo/EcalRecAlgos/src/EcalUncalibRecHitTimingCCAlgo.cc

RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerMultiFit.cc

RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerMultiFit.h

RecoLocalCalo/EcalRecAlgos/src/EcalUncalibRecHitTimingCCAlgo.cc

…go.h

thomreis

Second iteration.

RecoLocalCalo/EcalRecAlgos/interface/EcalUncalibRecHitTimingCCAlgo.h

RecoLocalCalo/EcalRecAlgos/src/EcalUncalibRecHitTimingCCAlgo.cc

RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerMultiFit.cc

thomreis · 2021-02-16T09:50:55Z

RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerMultiFit.cc

+     } else if (timealgo_ == kansasMethodCC) {
+
+        float startTime = -25;
+        float stopTime = 25;


ecalPh1::Samp_Period?

RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerMultiFit.cc

thomreis · 2021-02-17T13:39:10Z

RecoLocalCalo/EcalRecAlgos/src/EcalUncalibRecHitTimingCCAlgo.cc

+  if (counter < MIN_NUM_OF_ITERATIONS || counter > MAX_NUM_OF_ITERATIONS - 1) {
+    if (counter > MAX_NUM_OF_ITERATIONS / 2)
+      //Produce a log if minimization took too long
+      edm::LogWarning("EcalUncalibRecHitTimingCCAlgo::computeTimeCC")


How often do you expect this warning to happen?

not to often, actually, it never happened to me. But if it happens it is useful to know.
In general, this code was tested for functionality (time precision vs previous algorithms) but not for performance.
The average number of cycles here is ~5 but it could be reduced further and maybe something can be done to improve rare cases where it takes too long (MAX_NUM_OF_ITERATIONS)

Is it worth a warning or would logInfo do as well? Your call.

I'll leave just a logInfo, since in any case if this happens, the jitterError will be negative (to tell users not to trust the jitter for that rechit)

…ingCCAlgo obj created once)

RecoLocalCalo/EcalRecProducers/python/ecalMultiFitUncalibRecHit_cfi.py

RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerMultiFit.cc

nminafra · 2021-02-17T17:06:21Z

I've created a test python that should work (after voms-proxy-init):
RecoLocalCalo/EcalRecProducers/test/testEcalUncalibRechitProducerWithCC_cfg.py

RecoLocalCalo/EcalRecProducers/test/testEcalUncalibRechitProducerWithCC_cfg.py

thomreis · 2021-02-19T09:21:43Z

I think after the the test config is fixed, the standard matrix tests are passing, and when you have validated your test results this is ready for a PR to the official repo.

thomreis · 2021-02-19T09:22:06Z

Nice work. Thanks.

RecoLocalCalo/EcalRecAlgos/src/EcalUncalibRecHitTimingCCAlgo.cc

RecoLocalCalo/EcalRecProducers/test/testEcalUncalibRechitProducerWithCC_cfg.py

nminafra · 2021-02-19T16:05:33Z

Runing
$> runTheMatrix.py -l limited -i all --ibeos
gave no errors. Log file here:
https://nminafra.web.cern.ch/ECAL/runall-report-step123-.log

Co-authored-by: Slava Krutelyov <[email protected]>

thomreis · 2021-03-17T15:42:35Z

Closing this since PR cms-sw#33119 is now merged in master. Thanks for the nice work.

nminafra added 2 commits February 12, 2021 15:15

Rebasing kucc for 11_3

7ba20c8

Simplified inclusion of kucc

38506d2

thomreis reviewed Feb 12, 2021

View reviewed changes

nminafra added 5 commits February 13, 2021 10:44

Implemented thomreis suggestions iteration 1

a93f1a2

scram build code-format

c0bdba3

kansasMethodCC renamed as crossCorrelationMethod

a7b115f

added comment for magic constants from EcalUncalibRecHitTimeWeightsAl…

857a61c

…go.h

kansasMethodCC renamed as crossCorrelationMethod complete

4d1c340

thomreis reviewed Feb 17, 2021

View reviewed changes

Second round of thomreis comments

13690d6

thomreis reviewed Feb 17, 2021

View reviewed changes

nminafra added 2 commits February 17, 2021 15:46

Code cleanup and small performance optimization (EcalUncalibRecHitTim…

465da15

…ingCCAlgo obj created once)

Added parameters in python files

514e38d

nminafra commented Feb 17, 2021

View reviewed changes

RecoLocalCalo/EcalRecProducers/python/ecalMultiFitUncalibRecHit_cfi.py Show resolved Hide resolved

Added parameters to ParameterSetDescription

d183be0

nminafra commented Feb 17, 2021

View reviewed changes

RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerMultiFit.cc Show resolved Hide resolved

nminafra added 2 commits February 17, 2021 17:18

Adding test file dedicated to CC method

a74b1fc

working test file

75833b8

LogWarning to LogInfo when too many CC iterations

21901ad

thomreis reviewed Feb 19, 2021

View reviewed changes

RecoLocalCalo/EcalRecProducers/test/testEcalUncalibRechitProducerWithCC_cfg.py Show resolved Hide resolved

thomreis reviewed Feb 19, 2021

View reviewed changes

RecoLocalCalo/EcalRecAlgos/src/EcalUncalibRecHitTimingCCAlgo.cc Outdated Show resolved Hide resolved

nminafra added 3 commits February 19, 2021 13:56

slightly more efficient

3be47f2

Cleanup

a916b16

Cleanup of test

1e4a5df

thomreis reviewed Feb 19, 2021

View reviewed changes

RecoLocalCalo/EcalRecProducers/test/testEcalUncalibRechitProducerWithCC_cfg.py Outdated Show resolved Hide resolved

code-checks and code-format

252a9e4

nminafra and others added 28 commits March 10, 2021 08:04

Fixeing clang warnings

ad7d742

Removing destructor

ba9e43b

Co-authored-by: Slava Krutelyov <[email protected]>

Passing float by value

0179ab0

Co-authored-by: Slava Krutelyov <[email protected]>

Constexpr

e770087

Co-authored-by: Slava Krutelyov <[email protected]>

More efficient access to vector elements

4904d82

Co-authored-by: Slava Krutelyov <[email protected]>

C++ style cast

ddf38c2

Co-authored-by: Slava Krutelyov <[email protected]>

Using range loop

f5743f8

Co-authored-by: Slava Krutelyov <[email protected]>

Efficiency improvement

5d248d8

Co-authored-by: Slava Krutelyov <[email protected]>

Efficiency improvement

c7f9e7d

Co-authored-by: Slava Krutelyov <[email protected]>

Efficiency improvement

062cc95

Co-authored-by: Slava Krutelyov <[email protected]>

const added

81c6ce9

Co-authored-by: Slava Krutelyov <[email protected]>

const added

b81a2a7

Co-authored-by: Slava Krutelyov <[email protected]>

Const added

421fe2f

Co-authored-by: Slava Krutelyov <[email protected]>

rule 2.12

6af8364

Co-authored-by: Slava Krutelyov <[email protected]>

Avoid repeated computation inside the loop

03a441f

Co-authored-by: Slava Krutelyov <[email protected]>

rule 2.12

d29bd1b

Co-authored-by: Slava Krutelyov <[email protected]>

Constexpr

f21df41

Co-authored-by: Slava Krutelyov <[email protected]>

Efficiency improvement

4f60d30

Co-authored-by: Slava Krutelyov <[email protected]>

Reordered public/private

3d10143

Using fac,facM1orP2,facP1

ad0c604

Using fac,facM1orP2,facP1

44df65f

Cleaner test file

21cd746

Using unique_ptr for EcalUncalibRecHitTimingCCAlgo

cf995bc

Using single precision float

3f74b66

Removed log

cb78d71

code-format

8442f0b

Minor fixes

e47da18

errOnTime fix

e10760c

thomreis closed this Mar 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cross Correlation algorithm for ECAL timing, first implementation #30

Cross Correlation algorithm for ECAL timing, first implementation #30

nminafra commented Feb 12, 2021

thomreis left a comment

thomreis left a comment

thomreis Feb 16, 2021

thomreis Feb 17, 2021

nminafra Feb 17, 2021

thomreis Feb 17, 2021

nminafra Feb 19, 2021

nminafra commented Feb 17, 2021

thomreis commented Feb 19, 2021

thomreis commented Feb 19, 2021

nminafra commented Feb 19, 2021 •

edited

Loading

thomreis commented Mar 17, 2021

Cross Correlation algorithm for ECAL timing, first implementation #30

Cross Correlation algorithm for ECAL timing, first implementation #30

Conversation

nminafra commented Feb 12, 2021

thomreis left a comment

Choose a reason for hiding this comment

thomreis left a comment

Choose a reason for hiding this comment

thomreis Feb 16, 2021

Choose a reason for hiding this comment

thomreis Feb 17, 2021

Choose a reason for hiding this comment

nminafra Feb 17, 2021

Choose a reason for hiding this comment

thomreis Feb 17, 2021

Choose a reason for hiding this comment

nminafra Feb 19, 2021

Choose a reason for hiding this comment

nminafra commented Feb 17, 2021

thomreis commented Feb 19, 2021

thomreis commented Feb 19, 2021

nminafra commented Feb 19, 2021 • edited Loading

thomreis commented Mar 17, 2021

nminafra commented Feb 19, 2021 •

edited

Loading