-
Notifications
You must be signed in to change notification settings - Fork 16
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
Feature/ehinkle hip selection #97
base: develop
Are you sure you want to change the base?
Conversation
…swap for proto_nd_flow.
…al_hits hits dataset.
Feature/ehinkle eventdisplay
…Channels resource in proto_nd_flow.
…esources Feature/ehinkle module1 reflow add resources
…module1_flow input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Elise! Most of my comments are organizational and are just for you consideration. The major one that would be good to address before merging this PR is the one about the extra datasets in ndlar_flow/src/proto_nd_flow/reco/charge/calib_prompt_hits.py
, as that is code that we are using in the mainline 2x2 analysis chain.
@@ -0,0 +1,46 @@ | |||
################################################################################ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps a better place for this to live would be in ndlar_flow/src/proto_nd_flow/util
?
@@ -0,0 +1,57 @@ | |||
################################################################################ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps a better place for this to live would be in ndlar_flow/src/proto_nd_flow/util
?
@@ -60,6 +60,8 @@ class CalibHitBuilder(H5FlowStage): | |||
ts_pps f8, PPS packet timestamp [ns] | |||
io_group u8, io group ID (PACMAN number) | |||
io_channel u8, io channel ID (related to PACMAN number & PACMAN UART Number) | |||
chip_id u8, chip ID (ASIC number on PACMAN UART) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know you're using the chip_id and channel_id fields to debug without having to go through the references to packets, but can we remove these from the PR?
@@ -0,0 +1,757 @@ | |||
import numpy as np |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps a better place for this to live would be in ndlar_flow/src/proto_nd_flow/reco/combined
?
@@ -0,0 +1,545 @@ | |||
import numpy as np |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps a better place for this to live would be in ndlar_flow/src/proto_nd_flow/reco/combined?
@@ -0,0 +1,26 @@ | |||
classname: TrackletMerger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps a better place for this to live would be in ndlar_flow/yamls/proto_nd_flow/reco/combined
?
@@ -0,0 +1,25 @@ | |||
classname: TrackletReconstruction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps a better place for this to live would be in ndlar_flow/yamls/proto_nd_flow/reco/combined
?
merge_cut: 65 # merge hits with delta t < merge_cut [CRS ticks] | ||
max_merge_steps: 50 # max number of iterations when merging |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These seem like they are copied over from CalibHitMerger.yaml - do you need them here or can they be safely removed (just to keep things clean)?
@@ -0,0 +1,26 @@ | |||
# Generates the mid-level event built data for charge data (i.e. hits and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rename this file to be a bit more specific?
@@ -0,0 +1,23 @@ | |||
# Generates higher-level event built data for charge data (i.e. tracklets) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps a better place for this to live would be in ndlar_flow/yamls/proto_nd_flow/reco/combined
?
Adding tracklets (updated from module0_flow) and preliminary HIP selection code to proto_nd_flow. Also implemented are time-dependent disabled_channels resource and particle_data resource in proto_nd_flow.