Skip to content

Conversation

@lynnt20
Copy link
Contributor

@lynnt20 lynnt20 commented Nov 19, 2025

Description

Add producer module to create light calorimetry data products. Creates new subdirectory Calorimetry to store the module and fcls. Over 3 years since the first commit! Reco2/caf level changes only.

Checklist

  • Added at least 1 label from available labels.
  • Assigned at least 1 reviewer under Reviewers,
  • Assigned all contributers including yourself under Assignees
  • Linked any relevant issues under Developement
  • Does this PR affect CAF data format? If so, please assign a CAF maintainer (PetrilloAtWork or JosiePaton) as additional reviewer.
  • Does this affect the standard workflow?
  • Is this PR a patch for the ongoing production? If so, separate PR must also be made for production/v10_06_00 branch!

Relevant PR links

Accompanying PRs: SBNSoftware/sbncode#619, SBNSoftware/sbnanaobj#181, SBNSoftware/sbnobj#158

Link(s) to docdb describing changes (optional)

Is there a docdb describing the issue this solves or the feature added?

- cleaned (but need to clean more) skipping products that aren't used (xarapuca flashes + truth products) for example to avoid nullptr
- cleaned (but need to clean more) skipping products that aren't used (xarapuca flashes + truth products) for example to avoid nullptr
- moved truth validation to a separate function
- added lower/upper threshold for PE per channel
- added fcl param to turn on/off evaluating all planes
- option to add dir/refl efficiencies; NOTE: not really accounted for in the code yet
- merge the two previous trees into one
- match-type can now differentiate between event/slice level easily
Version v09_89_01, patch release for SBN2024A
Copy link
Contributor

@asanchezcastillo asanchezcastillo left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together @lynnt20, this is a lot of great work! I just left some minor comments, mainly about fcl inheritance which I think is strongly recommended in this case.

@lynnt20
Copy link
Contributor Author

lynnt20 commented Jan 28, 2026

Thanks for all the comments and suggestions @asanchezcastillo !! I think I've addressed everything, let me know if you see anything else!

@linyan-w
Copy link
Contributor

linyan-w commented Jan 28, 2026

Adding sim/reco group conveners for additional sign-off.
Please note that this is a new feature that is

  1. reco2 & downstream
  2. does not conflict with anything else

We will merge with any of the conveners sign-off on it from the physics & organization perspective. Lynn's CM presentation here:
https://sbn-docdb.fnal.gov/cgi-bin/sso/ShowDocument?docid=44953

Copy link
Contributor

@linyan-w linyan-w left a comment

Choose a reason for hiding this comment

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

Looks good to me! One nitpicking comment: Is the Noise Threshold and Max Threshold unique to light calorimetry? If so great. Otherwise it would be nice to check that's the globally used values.

@lynnt20
Copy link
Contributor Author

lynnt20 commented Jan 28, 2026

Looks good to me! One nitpicking comment: Is the Noise Threshold and Max Threshold unique to light calorimetry? If so great. Otherwise it would be nice to check that's the globally used values.

@linyan-w They are unique to this module, so no globally used values here.

Copy link
Member

@henrylay97 henrylay97 left a comment

Choose a reason for hiding this comment

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

In general this looks great @lynnt20! Thanks for the comprehensive approach. Also you should get an award for perseverance in not letting this branch die!

A few small comments, some of which can be ignored at your discretion.

@nathanielerowe
Copy link
Contributor

Can you resolve merge conflicts when you get a chance? Thanks!

@lynnt20 lynnt20 requested a review from sungbinoh January 29, 2026 20:38
@lynnt20
Copy link
Contributor Author

lynnt20 commented Jan 29, 2026

@sungbinoh, I added functionality to access TPC Calibration database to grab the electron lifetime for data. Would you mind reviewing that part of the code (producer module and data flc)? Thank you!

Copy link
Member

@henrylay97 henrylay97 left a comment

Choose a reason for hiding this comment

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

Thanks for the swift updates, looks great!


// Query database - translate run into fake "timestamp"
// (same convention as NormalizeDriftSQLite)
felifetime_db->UpdateData((run + 1000000000) * 1000000000);
Copy link
Member

Choose a reason for hiding this comment

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

General comment for all of us, and this can be punted to a separate PR. Should we have a single place where the fake "timestamp" is formed? So that when we inevitably end up changing the prescription in future we only have one point of maintenance.

Copy link
Member

Choose a reason for hiding this comment

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

@sungbinoh may have thoughts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: Waiting on Reviewer

Development

Successfully merging this pull request may close these issues.

6 participants