Skip to content

Add PMT Beam Signal timing to ICARUS CAFs #541

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

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

aheggest
Copy link
Contributor

@aheggest aheggest commented Jun 3, 2025

Description

This pull request reads the special PMT Beam timing signal (RWM) into the CAFMaker which enables us to reconstruct the Beam Bunch structure seen by ICARUS PMTs.

The PMT Beam signals were originally stored in an icaruscode data product icarus::timing::PMTBeamSignal, but icaruscode PR #832 and sbnobj PR #130 together move the data product to a SBN common area, sbnobj/Common.

A std::vector<sbn::timing::PMTBeamSignal> is read into the CAFMaker and then passed to FillICARUSOpFlash if the PMTBeamSignal is valid. If running on MC, offbeam data or stage1 files without the sbn::timing::PMTBeamSignal data product, the srflash.rwmtime variable will be left to it's default value kSignalingNaN in SROpFlash.h.

Within FillICARUSOpFlash, two new helper functions defined in sbnobj/Common/PMT/Data/PMTBeamSignal.cxx are called.

  • SelectFirstOpHitByTime reads in a single OpHit and returns a startmap and risemap (holding the start and rise time of each OpHit channel).
  • getFlashBunchTime reads in the risemap and std::vector<sbn::timing::PMTBeamSignal> RWMTimes, finds the mean between the first OpHit RiseTimes on opposite walls to get the Flash Interaction time wrt the RWM time.

This PR is a companion to sbnanaobj PR #141. This PR requires icaruscode PR #832 and sbnobj PR #130 also being merged.

  • Have you added a label? (bug/enhancement/physics etc.)
  • Have you assigned at least 1 reviewer?
  • Is this PR related to an open issue / project?
  • Does this PR affect CAF data format? If so, please assign a CAF maintainer as additional reviewer.
  • Does this PR require merging another PR in a different repository (such as sbnanobj/sbnobj etc.)? If so, please link it in the description.
  • Are you submitting this PR on behalf of someone else who made the code changes? If so, please mention them in the description.

Copy link
Member

@mvicenzi mvicenzi left a comment

Choose a reason for hiding this comment

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

Generally looks good. Main comment is: we need to make sure this doesn't break if the product is missing (MC files, older data files). Have you tested this as well?

@aheggest aheggest marked this pull request as draft June 3, 2025 18:16
@aheggest aheggest moved this to PR in progress in SBN software development Jun 3, 2025
@aheggest aheggest marked this pull request as ready for review June 6, 2025 19:56
Copy link
Member

@mvicenzi mvicenzi left a comment

Choose a reason for hiding this comment

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

My comments were addressed by the recent changes, so I approve.

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: PR in progress
Development

Successfully merging this pull request may close these issues.

2 participants