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

Runtime pT input + low pT occupancies (rebase PR39 and PR54) #130

Conversation

ariostas
Copy link
Member

@ariostas ariostas commented Nov 21, 2024

This is a rebase of PRs #39 and #54. They both deal with pt cutoffs, so I grouped them together.

One thing we have to think is whether adding the notebook would be okay for CMSSW since it's a relatively large file (2.4MB). I think maybe we should move it to another SegmentLinking repo or at least strip out the outputs.

@ariostas ariostas requested a review from GNiendorf November 21, 2024 19:47
@slava77
Copy link

slava77 commented Nov 21, 2024

This is a rebase of PRs #38 and #54.

I guess it should be #39

Does this already have conflicts with other rebases? I'm curious when we should start staging them out (merging to the devel_rebased ) branch

@ariostas
Copy link
Member Author

I guess it should be #39

Oops, fixed it.

Does this already have conflicts with other rebases? I'm curious when we should start staging them out (merging to the devel_rebased ) branch

I checked and there are no conflicts between them.

@GNiendorf
Copy link
Member

GNiendorf commented Nov 21, 2024

One thing we have to think is whether adding the notebook would be okay for CMSSW since it's a relatively large file (2.4MB). I think maybe we should move it to another SegmentLinking repo or at least strip out the outputs.

Yeah I agree with moving it to another repo, no reason to include it in CMSSW. Same with the DNN training notebook, but maybe other people have different opinions.

@slava77
Copy link

slava77 commented Nov 22, 2024

One thing we have to think is whether adding the notebook would be okay for CMSSW since it's a relatively large file (2.4MB). I think maybe we should move it to another SegmentLinking repo or at least strip out the outputs.

Yeah I agree with moving it to another repo, no reason to include it in CMSSW. Same with the DNN training notebook, but maybe other people have different opinions.

I checked on MM, it sounds like there is some support to have notebooks for documentation. ( I mentioned ~< 1MB size)

@ariostas
Copy link
Member Author

I checked on MM, it sounds like there is some support to have notebooks for documentation. ( I mentioned ~< 1MB size)

I tried using ipynbcompress to reduce the resolution of the images to get it down to ~300KB. The images are pretty blurry, but you can still make out the important features. Alternatively, we could just remove all images.

https://gist.github.com/ariostas/91c12257271a72cbd2245a0a9d99043b

Co-authored-by: Gavin Niendorf <[email protected]>
@ariostas ariostas changed the title Rebase PR39 and PR54 Runtime pT input + low pT occupancies (rebase PR39 and PR54) Nov 22, 2024
@ariostas
Copy link
Member Author

/run all

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     40.6    384.4    191.7    128.3    135.1    500.5    117.7    245.1    102.0      2.9    1848.2    1307.2+/- 365.6     491.0   explicit[s=4] (target branch)
   avg     40.5    388.4    183.1    127.2    136.0    546.7    118.5    246.3    108.4      3.0    1898.1    1310.9+/- 365.8     501.2   explicit[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.

@GNiendorf GNiendorf merged commit ac2e345 into CMSSW_14_1_0_pre3_LST_X_LSTCore_realfiles_batch1_devel_rebased Nov 25, 2024
3 checks passed
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.

3 participants