Skip to content
This repository has been archived by the owner on Dec 9, 2024. It is now read-only.

Move to Binary Data Files + Include 0.6 pT Maps #391

Merged
merged 7 commits into from
Apr 24, 2024
Merged

Conversation

GNiendorf
Copy link
Member

@GNiendorf GNiendorf commented Apr 22, 2024

This PR moves the data files to a binary format and adds the 0.6 pT maps into the data dir.

Followup PR of SegmentLinking/LSTGeometry#11

@SegmentLinking SegmentLinking deleted a comment from github-actions bot Apr 22, 2024
@@ -145,7 +145,7 @@ namespace SDL {

// Temporary fix for module buffer allocation.
const unsigned int modules_size = 26401;
const unsigned int pix_tot = 1794686;
const unsigned int pix_tot = 1795336;
Copy link
Member Author

Choose a reason for hiding this comment

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

https://github.com/SegmentLinking/LSTGeometry/blob/15f482747748e89632fbef65989355ddbb8d94d7/Constants.py#L17

I'm pretty sure this small change comes from my last PR on LSTGeometry where I consolidated the constants and changed B to be 3.8112 from 3.8 to keep it consistent with the Tracklooper definition.

@GNiendorf
Copy link
Member Author

/run standalone
/run cmssw

@GNiendorf GNiendorf marked this pull request as ready for review April 22, 2024 22:16
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.

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.

@SegmentLinking SegmentLinking deleted a comment from github-actions bot Apr 23, 2024
@SegmentLinking SegmentLinking deleted a comment from github-actions bot Apr 23, 2024
@GNiendorf GNiendorf requested a review from ariostas April 23, 2024 21:11
@GNiendorf
Copy link
Member Author

/run standalone
/run cmssw

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.

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.

Copy link
Member

@ariostas ariostas left a comment

Choose a reason for hiding this comment

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

Thank you @GNiendorf, this is great! All the changes look good to me and everything is working well. It's really nice to not have a bunch of giant text files floating around.

I'll approve the PR and merge it in a few hours in case anyone else has any comments.

This is beyond the scope of this PR, but I wanted to ask you what you would think about consolidating the data into a single binary file. Using our own format like in this PR would be too messy, but it could be done with a ROOT file. The benefit I can think of is having a single smaller file (since it's compressed), but it would add extra complexity so it could be just an extra nuisance.

@GNiendorf
Copy link
Member Author

Thank you @ariostas. I like your suggestion of moving everything to a single file, although that's probably a lot of work for what it's worth right now. Could be a good project though for a new student or later down the line if this becomes a larger issue.

@ariostas ariostas merged commit 82d9fe3 into master Apr 24, 2024
3 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants