Precise RPCTime in muons #18681

jhgoh · 2017-05-10T20:20:33Z

The RPCTime information is computed based on the bunch crossing x 25ns, not taking the precise timing information which is available from the phase-II upgrade.

With this PR, the RPCTime in the reco::Muon is computed using the precise time measurement if available.

@ptraczyk

cmsbuild · 2017-05-10T20:20:52Z

A new Pull Request was created by @jhgoh (Junghwan John Goh) for master.

It involves the following packages:

RecoMuon/MuonIdentification

@perrotta, @cmsbuild, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@bellan, @abbiendi, @echapon, @calderona, @HuguesBrun, @battibass, @trocino, @bachtis, @rociovilar this is something you requested to watch as well.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

slava77 · 2017-05-10T20:26:11Z

@cmsbuild please test

cmsbuild · 2017-05-10T20:26:25Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19738/console Started: 2017/05/10 22:44

cmsbuild · 2017-05-10T22:02:03Z

+1
Tested at: 12e9ccf
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18681/19738/summary.html

cmsbuild · 2017-05-10T22:02:06Z

Comparison job queued.

cmsbuild · 2017-05-11T00:30:31Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18681/19738/summary.html

Comparison Summary:

You potentially added 9 lines to the logs
ROOTFileChecks: Some differences in event products or their sizes found
Reco comparison results: 1752 differences found in the comparisons
DQMHistoTests: Total files compared: 24
DQMHistoTests: Total histograms compared: 1830338
DQMHistoTests: Total failures: 22679
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 1807479
DQMHistoTests: Total skipped: 180
DQMHistoTests: Total Missing objects: 0
Checked 98 log files, 14 edm output root files, 24 DQM output files

perrotta · 2017-05-15T10:58:09Z

In this PR the RPCTime is filled with the precise timing information if not
hitRPC->timeError() < 0

If I read correctly, timeError is initialized to 0 in the RPCRecHit constructor (DataFormats/RPCRecHit/src/RPCRecHit.cc) and it is only modified if timeError>=0: https://cmssdt.cern.ch/lxr/source/RecoLocalMuon/RPCRecHit/src/RPCRecHitBaseAlgo.cc#0043

If so, the bx should never be used here, not even for rpc in phase0/1: could you please check?

(If my observation is correct, I would expect this PR may affect also phase0/1 reco, and not only phase 2. On the other hand, I can't see any modification in jenkins outputs, not even for RPC in phase 2: could you please suggest me which distribution I should look at, and possibly also provide yourself here some evidence that this update behaves as you want, and does not affect current reco)?

slava77 · 2017-05-15T12:56:51Z

On 5/15/17 3:58 AM, perrotta wrote: In this PR the RPCTime is filled with the precise timing information if not hitRPC->timeError() < 0 If I read correctly, timeError is initialized to 0 in the RPCRecHit constructor (DataFormats/RPCRecHit/src/RPCRecHit.cc) and it is only modified if timeError>=0: https://cmssdt.cern.ch/lxr/source/RecoLocalMuon/RPCRecHit/src/RPCRecHitBaseAlgo.cc#0043 If so, the bx should never be used here, not even for rpc in phase0/1: could you please check? (If my observation is correct, I would expect this PR may affect also phase0/1 reco, and not only phase 2. On the other hand, I can't see any modification in jenkins outputs, not even for RPC in phase 2:

what about this https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_9_2_X_2017-05-09-2300+18681/19931/validateJR/all_OldVSNew_RunPhoton2012Bwf4p53/ ?

…

could you please suggest me which distribution I should look at, and possibly also provide yourself here some evidence that this update behaves as you want, and does not affect current reco)? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#18681 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEdcbhMs7qku-j6KXia3AYUHEDc-JY_8ks5r6C_BgaJpZM4NXLCA>.

…ault is (0,-1)

cmsbuild · 2017-05-15T12:59:56Z

Pull request #18681 was updated. @perrotta, @cmsbuild, @slava77, @davidlange6 can you please check and sign again.

jhgoh · 2017-05-15T13:00:55Z

@perrotta Thank you for checking details. I missed the point.
I'm updating this PR to set the default values of time and its error - time and its error are initialized to be (0,-1) by default.

-> Use bx based time if timeError < 0
-> Use precise time if timeError >= 0

perrotta · 2017-05-15T13:06:49Z

please test

cmsbuild · 2017-05-15T13:07:09Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19831/console Started: 2017/05/15 15:12

cmsbuild · 2017-05-15T14:41:31Z

+1
Tested at: 85045c0
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18681/19831/summary.html

cmsbuild · 2017-05-15T14:41:36Z

Comparison job queued.

perrotta · 2017-05-15T14:54:57Z

Your latest commit is also going to affect (fix, I'd say...) validations:
https://cmssdt.cern.ch/lxr/source/Validation/RPCRecHits/src/RPCRecHitValid.cc#0545

In fact, that line was merged with #18632 five days ago, so probably nobody still checked its outcome. It would be interesting to see how the fix implemented here will act on RPC time plots (I expect that you get now some result different from 0 also for phase1/0)

jhgoh · 2017-05-15T15:44:22Z

The timing of RPCRecHits are new variables from 92X (91X backport is not merged yet). So, we did not have a chance to make comparisons yet.

What do I expect with this change in Phase1/0?

Change of RPCRecHit::timeError(), always set to -1 (no RPC with precise timing, therefore all RPCRecHit should have the default value)
No change in muons - RPC timing not used in tracking, identification.
No change of muon::RPCTime(), this was always =25ns x bx. And even wit h this PR, this should remain same as before for phase1/0 since there's no RPC with precise timing therefore all RPCRecHits will have timeError<0 and it will just use (25ns x bx).

cmsbuild · 2017-05-15T20:58:31Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18681/19831/summary.html

Comparison Summary:

You potentially added 8 lines to the logs
Reco comparison results: 1706 differences found in the comparisons
DQMHistoTests: Total files compared: 24
DQMHistoTests: Total histograms compared: 1830730
DQMHistoTests: Total failures: 23699
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 1806851
DQMHistoTests: Total skipped: 180
DQMHistoTests: Total Missing objects: 0
Checked 98 log files, 14 edm output root files, 24 DQM output files

perrotta · 2017-05-16T14:51:57Z

With the latest fix, now RPC time only gets modified for Phase 2, e.g. (TTbar14TeV2023D4):

Corresponding entries remain unchanged in Phase1 RPC, as it should.

As expected, the DQM plots for RPC are modified also in Phase 1, but this is simply due to the bug fix applied here which avoids having all rpcTIme=0 at phase 1 (as it was after #18632, see above #18681 (comment). Example for 10024.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017:

Thus everything goes as expected, now.

I remain with only one doubt: does hitRPC->time() refer to an "absolute time" computed with respect to BX=0, or does it refer to the time diff with respect to hitRPC->BunchX() instead?
I am asking because in the new distributions of rpcTIme I see only values close to 0 ns, while there are cases in which the BunchX() stays some N times 25 ns apart. If hitRPC->time() is the difference with the time of the corresponding BX, then probably BX times 25 should be added to it when computing
const double time = hitRPC->timeError() < 0 ? hitRPC->BunchX()*25. : hitRPC->time();

@jhgoh : could you please verify and let us know?

jhgoh · 2017-05-16T15:47:32Z

@perrotta thank you for comments and providing comparison plots.
The time from the RPCRecHit is absolute one. You can see timing distribution below, which gives peaks in every 25ns spacing:

This plot was done using the pre3 ZMM relval sample with PU200, the time is directly taken from the RPCRecHit.

perrotta · 2017-05-16T16:36:27Z

+1
Given #18681 (comment) and #18681 (comment)
Precise rpc timing is used, when available, instead of bx. It does not affect Phase0/1 reco

cmsbuild · 2017-05-16T16:36:46Z

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. @davidlange6, @smuzaffar

davidlange6 · 2017-05-17T07:47:00Z

+1

fill RPCTime with precise timing

12e9ccf

cmsbuild added this to the CMSSW_9_2_X milestone May 10, 2017

cmsbuild added comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels May 10, 2017

cmsbuild added tests-started and removed tests-pending labels May 10, 2017

cmsbuild added tests-approved and removed tests-started labels May 10, 2017

cmsbuild added comparison-available and removed comparison-pending labels May 11, 2017

change default RPCRecHit timeError and always set time and error, def…

85045c0

…ault is (0,-1)

cmsbuild added comparison-pending tests-pending and removed comparison-available tests-approved labels May 15, 2017

cmsbuild added tests-started and removed tests-pending labels May 15, 2017

cmsbuild added tests-approved and removed tests-started labels May 15, 2017

cmsbuild added comparison-available and removed comparison-pending labels May 15, 2017

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels May 16, 2017

cmsbuild added orp-approved and removed orp-pending labels May 17, 2017

cmsbuild merged commit f0f6d44 into cms-sw:master May 17, 2017

jhgoh deleted the muonIRPCTime branch September 26, 2017 02:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Precise RPCTime in muons #18681

Precise RPCTime in muons #18681

jhgoh commented May 10, 2017 •

edited

cmsbuild commented May 10, 2017

slava77 commented May 10, 2017

cmsbuild commented May 10, 2017 •

edited

cmsbuild commented May 10, 2017

cmsbuild commented May 10, 2017

cmsbuild commented May 11, 2017

perrotta commented May 15, 2017

slava77 commented May 15, 2017 via email

cmsbuild commented May 15, 2017

jhgoh commented May 15, 2017 •

edited

perrotta commented May 15, 2017

cmsbuild commented May 15, 2017 •

edited

cmsbuild commented May 15, 2017

cmsbuild commented May 15, 2017

perrotta commented May 15, 2017

jhgoh commented May 15, 2017

cmsbuild commented May 15, 2017

perrotta commented May 16, 2017

jhgoh commented May 16, 2017

perrotta commented May 16, 2017

cmsbuild commented May 16, 2017

davidlange6 commented May 17, 2017

Precise RPCTime in muons #18681

Precise RPCTime in muons #18681

Conversation

jhgoh commented May 10, 2017 • edited

cmsbuild commented May 10, 2017

slava77 commented May 10, 2017

cmsbuild commented May 10, 2017 • edited

cmsbuild commented May 10, 2017

cmsbuild commented May 10, 2017

cmsbuild commented May 11, 2017

perrotta commented May 15, 2017

slava77 commented May 15, 2017 via email

cmsbuild commented May 15, 2017

jhgoh commented May 15, 2017 • edited

perrotta commented May 15, 2017

cmsbuild commented May 15, 2017 • edited

cmsbuild commented May 15, 2017

cmsbuild commented May 15, 2017

perrotta commented May 15, 2017

jhgoh commented May 15, 2017

cmsbuild commented May 15, 2017

perrotta commented May 16, 2017

jhgoh commented May 16, 2017

perrotta commented May 16, 2017

cmsbuild commented May 16, 2017

davidlange6 commented May 17, 2017

jhgoh commented May 10, 2017 •

edited

cmsbuild commented May 10, 2017 •

edited

jhgoh commented May 15, 2017 •

edited

cmsbuild commented May 15, 2017 •

edited