SegmentLinking / TrackLooper

Apache License 2.0
5 stars 14 forks source link

T3 remove redundant new #393

Closed YonsiG closed 6 months ago

YonsiG commented 6 months ago

In the T3 kernels, we have some functions called runTripletDefaultAlgoBBBB, and also the BBEE and EEEE regions. Those cuts are duplicating with the passPointingConstraintBBB ... functions. On the other hand, introducing beta iteration cuts to triplet objects are not physically meaningful. In this PR, we still keep the first beta cut as geometric requirement and move to passPointingConstraint function, but remove all the beta iteration functions.

Another improvement is that the triplet radius is calculated and saved for each of the T3s. In this case, we don't need to calculate the radius of triplet multiple times when building T5, pT3 and pT5s. This will also bring timing improvement. Slides describing the influence of this PR is summarized here. https://indico.cern.ch/event/1407246/contributions/5914814/attachments/2846057/4976175/check%20the%20T3%20cuts%20EEEE%202.pdf

YonsiG commented 6 months ago

closed the previous PR with thread divergence. created a new one. Addressing Slava's first comments already.

YonsiG commented 6 months ago

/run standalone /run CMSSW

github-actions[bot] commented 6 months ago

The PR was built and ran successfully with CMSSW. Here are some plots.

OOTB All Tracks
Efficiency and fake rate vs pT, eta, and phi

The full set of validation and comparison plots can be found here.

YonsiG commented 6 months ago

Merging this PR will be able to close Issue #214 and Issue #247 related to triplet building cuts

YonsiG commented 6 months ago

/run standalone

github-actions[bot] commented 6 months ago

The PR was built and ran successfully in standalone mode. Here are some of the comparison plots.

Efficiency vs pT comparison Efficiency vs eta comparison
Fake rate vs pT comparison Fake rate vs eta comparison
Duplicate rate vs pT comparison Duplicate rate vs eta comparison

The full set of validation and comparison plots can be found here.

Here is a timing comparison:

   Evt    Hits       MD       LS      T3       T5       pLS       pT5      pT3      TC       Reset    Event     Short             Rate
   avg     43.8    323.0    122.9     69.7     91.5    492.8    128.5    157.5     98.3      3.3    1531.3     994.7+/- 267.6     418.3   explicit_cache[s=4] (master)
   avg     44.2    323.5    121.8     49.1     97.0    496.5    133.2    159.4    100.2      2.7    1527.5     986.9+/- 262.4     416.0   explicit_cache[s=4] (this PR)
YonsiG commented 6 months ago

This is the timing summary running on cgpu-1 after the current change.

Screenshot 2024-05-01 at 6 16 20 PM

This is the timing summary on cgpu-1 of the master.

Screenshot 2024-05-01 at 5 26 54 PM

The timing reduction of T3 kernels is ~20%. Due to moving radius calculation from T5 to T3, the T5 kernel time is also reduced by ~12%.

slava77 commented 6 months ago

This is the timing summary running on cgpu-1 after the current change.

please rerun to check that s=8 is just a fluctuation

YonsiG commented 6 months ago

This is the timing summary running on cgpu-1 after the current change.

please rerun to check that s=8 is just a fluctuation

Yes, I updated the previous comment by replacing the timing info

slava77 commented 6 months ago

/run standalone /run cmssw

github-actions[bot] commented 6 months ago

The PR was built and ran successfully in standalone mode. Here are some of the comparison plots.

Efficiency vs pT comparison Efficiency vs eta comparison
Fake rate vs pT comparison Fake rate vs eta comparison
Duplicate rate vs pT comparison Duplicate rate vs eta comparison

The full set of validation and comparison plots can be found here.

Here is a timing comparison:

   Evt    Hits       MD       LS      T3       T5       pLS       pT5      pT3      TC       Reset    Event     Short             Rate
   avg     43.1    323.0    124.8     71.1     93.3    544.9    129.6    159.2    105.1      3.1    1597.3    1009.3+/- 266.4     436.8   explicit_cache[s=4] (master)
   avg     45.6    323.4    123.7     50.6     98.5    548.8    135.9    158.9    108.9      2.4    1596.6    1002.2+/- 264.6     433.4   explicit_cache[s=4] (this PR)
github-actions[bot] commented 6 months ago

The PR was built and ran successfully with CMSSW. Here are some plots.

OOTB All Tracks
Efficiency and fake rate vs pT, eta, and phi

The full set of validation and comparison plots can be found here.

slava77 commented 6 months ago

/run standalone /run CMSSW

github-actions[bot] commented 6 months ago

The PR was built and ran successfully in standalone mode. Here are some of the comparison plots.

Efficiency vs pT comparison Efficiency vs eta comparison
Fake rate vs pT comparison Fake rate vs eta comparison
Duplicate rate vs pT comparison Duplicate rate vs eta comparison

The full set of validation and comparison plots can be found here.

Here is a timing comparison:

   Evt    Hits       MD       LS      T3       T5       pLS       pT5      pT3      TC       Reset    Event     Short             Rate
   avg     43.5    323.7    122.9     70.6     93.7    545.5    131.4    157.0    104.4      3.0    1595.7    1006.7+/- 268.6     435.2   explicit_cache[s=4] (master)
   avg     45.0    323.6    122.1     48.5     97.4    545.1    134.4    157.4    105.7      2.6    1581.9     991.8+/- 267.2     432.5   explicit_cache[s=4] (this PR)
github-actions[bot] commented 6 months ago

The PR was built and ran successfully with CMSSW. Here are some plots.

OOTB All Tracks
Efficiency and fake rate vs pT, eta, and phi

The full set of validation and comparison plots can be found here.

GNiendorf commented 6 months ago

Saw this warning while compiling, is this okay? Warning is maybe-uninitialized, not sure if it was discussed already. Screenshot 2024-05-10 at 10 30 26 PM

YonsiG commented 6 months ago

Saw this warning while compiling, is this okay? Warning is maybe-uninitialized, not sure if it was discussed already. Screenshot 2024-05-10 at 10 30 26 PM

Thank you for spotting this! I did a check and adds a printout just before line 203, and it can print meaningful values for the betaIn variable. And to be saved to the triplet collection, it has to pass through all the requirements, so for a good triplet the betaIn should be calculated and saved. To be safe, I can add initialization to betaIn as 0, as an extra protection for this.