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

Make compilation flags for duplicate cleaning and triplet pLSs into runtime flags #408

Closed
wants to merge 3 commits into from

Conversation

VourMa
Copy link
Contributor

@VourMa VourMa commented May 29, 2024

As per title. The default behavior should not change, as it will be (hopefully) demonstrated by the tests, while I have run comparisons for the application of the flags, and the results are as expected:

Master vs. no duplicate cleaning
https://uaf-10.t2.ucsd.edu/~evourlio/SDL/PR408/runtimeFlags__VS_2_enabled_50fb47D-PU200_50fb47D-PU200/summary/

Master vs. triplet pLSs in TCs
https://uaf-10.t2.ucsd.edu/~evourlio/SDL/PR408/runtimeFlags__VS_3_enabled_50fb47D-PU200_50fb47D-PU200/summary/

It goes together with the corresponding PR in cmssw (SegmentLinking/cmssw#31).

SDL/LST.h Outdated
Comment on lines 12 to 14
#include "Event.h"

#include "Math/Vector3D.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

can these be avoided?
Especially the first one; to keep the dependencies cleaner.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These were not added, they were just moved from the .cc to the .h file. Unless the movement is the problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unless the movement is the problem.

yes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I will revert.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted in 37b465a. I also included back the explicit SDL/Event.h dependency for sdl.h to make it clearer, even though it is implicitly included through write_sdl_ntuple.h.

@slava77
Copy link
Contributor

slava77 commented May 30, 2024

/run standalone
/run cmssw 31

Copy link

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     44.9    322.1    122.5     48.4     97.3    497.6    133.0    155.3     98.6      2.2    1522.0     979.5+/- 260.9     415.7   explicit_cache[s=4] (master)
   avg     47.1    325.1    123.8     48.5     97.9    507.1    134.7    157.4    106.1      2.5    1550.1     995.9+/- 264.8     424.9   explicit_cache[s=4] (this PR)

Copy link

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.

@VourMa
Copy link
Contributor Author

VourMa commented Jun 8, 2024

Superseded by SegmentLinking/cmssw#37.

@VourMa VourMa closed this Jun 8, 2024
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