Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix inventStubs bug in duplicate removal #271

Merged
merged 4 commits into from
Apr 8, 2024

Conversation

tomalin
Copy link
Collaborator

@tomalin tomalin commented Apr 8, 2024

When using L2L3D1 seeds, it is only L2L3 that are used to determined the r-z helix params, not L2D1 as the code currently assumes. This PR fixes this.

When run on ttbar+200PU, remaking the stubs using the CMSSW 13 stub window sizes, it reduces the track rate per event from 323 to 284.

When run on ttbar+200PU, using the existing stubs made with CMSSW 14 stub window sizes, it reduces the track rate per event from 294 to 268.

The efficiency on displaced muon MC is very slightly higher.

P.S. One can query if https://github.com/cms-L1TK/cmssw/blob/ianFixInventStubs/L1Trigger/TrackFindingTracklet/src/PurgeDuplicate.cc#L743 should be calling getInventedCoordsExtended() for the prompt seed types (0-7) that form part of the extended tracking. Though I think getInventedCoordsExtended() and getInventedCoords() should give the same results for these.

tomalin added 4 commits April 8, 2024 15:00
When using L2L3D1 seeds, it is only L2L3 that are used to determined the r-z helix params, not L2D1 as the code currently assumes.
@tomalin tomalin requested a review from sarafiorendi April 8, 2024 15:50
@tomalin tomalin marked this pull request as ready for review April 8, 2024 15:50
Copy link

@sarafiorendi sarafiorendi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, it fixes the bug indeed

@tomalin tomalin merged commit f4e51ce into L1TK-dev-14_0_0_pre2 Apr 8, 2024
1 check passed
aryd added a commit that referenced this pull request Jun 11, 2024
* Andrews KF crash fix (#263)

* Add z0 resolution to performance printout (#264)

* Add z0 resolution to performance printout

* code format

* DR: fix displaced track bug & disable binning (#266)

* Disable binning in DR

* bug fix

* add comment

* code format

* tweak comment

* fix previous erroneous commit

* DUMMY COMMIT BEFORE PR TO CENTRAL CMSSW

* Run combined modules by default (#265)

* Make combined modules default

* tweak

* Improve USEHYBRID ifdef range

* Fix compiler error for pure Tracklet algo

* Move fitpattern.txt refs, so only used for pure Tracklet algo

* code format

* Numerical stability fix (#269)

* Made calculations more numerically stable.

* Explicitly restrict to domain of asin/acos.

* Added comments.

* Code format.

* Fixed typos in comments.

* Only do calculations when needed.

* Added const where applicable.

* Fix inventStubs bug in duplicate removal (#271)

* Fix inventStubs bug in duplicate removal

When using L2L3D1 seeds, it is only L2L3 that are used to determined the r-z helix params, not L2D1 as the code currently assumes.

* Update PurgeDuplicate.cc

* add comment

* formatted

* Manually incorporating DTC stub, TT stub changes to CMSSW 14 dev branch

* First pass at removing non-combined modules

* Removed unused iMath code

* Avoid stale pointers on subsequent events

* Cleaner MP pointer checks

Full agreement with HLS (100 events L1PHIC - D5PHIC)

* First code for a ProjectionCalculator module

* Set of changes to make a configuration that uses the Projection Calculator module

* Fix mistake in code merging

* Changes needed for the VMSME Router module

* Fully implementing VMSMERouter, VMRouterCM no longer producing VMMEStubs

* Manually incorporating DTC stub, TT stub changes to CMSSW 14 dev branch

* Readding TCBase, VMSMER compatible w/ updated stub format

* Addressing comments, fixing code format

* Removing unneeded diskpswrittenr variable

* Removing unneeded + 1 to rbits in TP

* Updates to make sure we don't have missing projections

---------

Co-authored-by: Ian Tomalin <[email protected]>
Co-authored-by: Andrew Hart <[email protected]>
Co-authored-by: Anders <[email protected]>
Co-authored-by: bryates <[email protected]>
@tomalin tomalin deleted the ianFixInventStubs branch July 18, 2024 10:15
dally96 pushed a commit that referenced this pull request Dec 3, 2024
* Fix inventStubs bug in duplicate removal

When using L2L3D1 seeds, it is only L2L3 that are used to determined the r-z helix params, not L2D1 as the code currently assumes.

* Update PurgeDuplicate.cc

* add comment

* formatted
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants