[PPS] Timing detectors tracks window reduced to 25 ns in lite tracks format

[forthommel]

PR description:

The size of the time slice for tracks in PPS diamond timing detectors is reduced to the one 25 ns window of interest at the miniAOD lite data format level.

PR validation:

Compiles, and runs for the 2017-2018 periods where PPS diamond were in place and calibrated.

if this PR is a backport please specify the original PR: N/A

Before submitting your pull requests, make sure you followed this checklist:

• verify that the PR is really intended for the chosen branch
• verify that changes follow CMS Naming, Coding, And Style Rules
• verify that the PR passes the basic test procedure suggested in the CMSSW PR instructions

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26607/9551

• This PR adds an extra 16KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

• code-format:
  https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26607/9551/code-format.patch
  e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26607/9551/code-format.patch | patch -p1
  You can also run scram build code-format to apply code format directly

[cmsbuild]

The code-checks are being triggered in jenkins.

[forthommel]

Sorry, thanks to the code-format this PR went from a very simple and direct fix to a huge coding convention change...

[cmsbuild]

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26607/9558

• This PR adds an extra 20KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

• code-format:
  https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26607/9558/code-format.patch
  e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26607/9558/code-format.patch | patch -p1
  You can also run scram build code-format to apply code format directly

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26607/9671

• This PR adds an extra 24KB to repository

[cmsbuild]

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

[slava77]

and another case of commits with timestamps after the tests were done.

@smuzaffar @mrodozov @fabiocos
should I restart the tests or will the relabeling be fixed in the bot?

[forthommel]

The size of the time slice for tracks in PPS diamond timing detectors is reduced to the one 25 ns window of interest at the miniAOD lite data format level.

please add a link to some slides to justify this selection.
Is the new selection implying that the job is running on a newly calibrated data (106X release reco inputs)?

Would this +/-12.5 ns selection still work for the data made in earlier releases (e.g. the original Run2 reco, 80X , 94X, or 102X)? If no, some appropriate modifiers should be applied to keep the right selection.

Hi @slava77
Sorry for the radio silence, I am currently busy with another project these days. You mean there is a possibility this lite tracks collection producer will be re-ran in the future with legacy releases?
By the way, this skimming is only meaningful from 10_6_X on, as the timing detectors' track timing calculation was introduced with #26327 (previously CTPPSDiamondLocalTrack input collection had t=0).
Now, for the quantitative study of the overall size reduction, I may try to produce this for 2017 as for 2018 this range is not changing anything. This +- 12.5 ns window opening corresponds to one single acquisition window (centered on 0 with the use the timing calibration introduced earlier, see e.g. in slide 5 of this AlCaDB presentation). In 2017 we were collecting 5, 3, 2, and progressively 1x25 ns time windows, and remained with 1 in 2018.

[slava77]

On 5/8/19 7:16 AM, Laurent Forthomme wrote: Sorry for the radio silence, I am currently busy with another project these days. You mean there is a possibility this lite tracks collection producer will be re-ran in the future with legacy releases?

yes, it is quite possible.

track timing calculation was introduced with #26327 (previously

CTPPSDiamondLocalTrack input collection had t=0). the cut is applied on t + OOT*25. Is the OOT unchanged by the calibrations? Should we expect more or less events passing without the calibration applied (i.e with the cut applied to "0 + OOT*25" as would be read from the older data) ?

see e.g. in slide 5 of this AlCaDB presentation).

from this and perhaps p4 I would conclude that the proposed cut is going to be safe for all data, even uncalibrated. Please confirm (if confirmed, this PR can just be signed as is).

[slava77]

it looks like github.com is continuing to go crazy
my last comment from 30 mins ago is appearing earlier than my comment from 5 hours ago
#26607 (comment)

I'm re-quoting it here for a more linear reading

On 5/8/19 7:16 AM, Laurent Forthomme wrote: Sorry for the radio silence, I am currently busy with another project these days. You mean there is a possibility this lite tracks collection producer will be re-ran in the future with legacy releases?

yes, it is quite possible.

track timing calculation was introduced with #26327 (previously
CTPPSDiamondLocalTrack input collection had t=0).

the cut is applied on t + OOT25. Is the OOT unchanged by the calibrations? Should we expect more or less events passing without the calibration applied (i.e with the cut applied to "0 + OOT25" as would be read from the older data) ?

see e.g. in slide 5 of this AlCaDB presentation).

from this and perhaps p4 I would conclude that the proposed cut is going to be safe for all data, even uncalibrated. Please confirm (if confirmed, this PR can just be signed as is).

[forthommel]

@slava77

Is the OOT unchanged by the calibrations?

Yes, it is only propagated from rechits level information

Should we expect more or less events passing without the calibration
applied (i.e with the cut applied to "0 + OOT*25" as would be read from the older data) ?

If one considers older releases (no timing reconstruction), t = 0 falls 100% of the time within this time window selection. For early 2017 data (before run 302159 where multiple OOT ranges are expected), an overall reduction. Unchanged elsewhere.

from this and perhaps p4 I would conclude that the proposed cut is going to be safe for all data, even uncalibrated. Please confirm (if confirmed, this PR can just be signed as is).

👍

[slava77]

+1

for #26607 6033dd3

• code changes are in line with the PR description and the follow up review
• jenkins tests pass and comparisons with the baseline show no differences (this, however, is because there is no data with ctppsDiamondLocalTracks used in the tests)
  • the signoff is based mostly on visual inspection of somewhat trivial change in the track selection logic.

[cmsbuild]

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 will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

[fabiocos]

+1