-
Notifications
You must be signed in to change notification settings - Fork 29
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
In compatibility of the light backtracking segment info and event / trigger info #205
Comments
Regarding trigger ID, inconsistent call to export_light_wvfm_to_hdf5 between here (correct) and here (bug) |
@russellphysics Could you elaborate please? |
"i_trig" not provided in the function call at the last link |
Ah! Yes! You are absolutely right. Testing now. |
I don't fully grasp the difference and relationship between event ID and light trigger ID; regardless, the hard coding of event_id[0] is likeley suspect EDIT: ignore this comment. This assumption is ok... based on this |
While implementing the fix Brooke pointed out, I realised it shouldn't affect the current output... as the error is in a chunk that uses threshold trigger. |
Manual check of the input to the export_to_hdf5 function in simulate_pixels.py (for all four modules, no segments found that are in more than module/event_id/trigger_id). So, the input appears to be as expected. Example output used for checking:
Print statements were inserted in simulate_pixels.py before:
I printed results['light_event_id'], i_trig and results['light_waveforms_true_track_id']. I also tested the np.ndenumerate line in the zero_suppress function. This appears to work as expected. The indexing is also done correctly I believe |
If mod2mod variations are turned off, the issue does not appear |
Maybe related, maybe a separate issue, but only half of the op_channel_ids seem to be there. |
Are we missing an indent in this line (and the following ones?) larnd-sim/larndsim/light_sim.py Line 515 in 785fbbb
And should i_det be i_det_signal here larnd-sim/larndsim/light_sim.py Line 537 in 785fbbb
|
The op_channel_id array actually looks correct: each ADC unit per TPC has 96 channels; however only 48 of them are actually required for the 2x2. We have some spare/extra channels |
I don't understand? 48 channels per tpc (because twice 2 arclights with 6 sipms and 6 lcms with 2 sipms) would be 96 per module, so 384 in total right? I only see half of that. It shows 0-47 and then 96-143 etc. |
You don't understand because I'm saying the wrong thing! Each ADC unit per module has 96 channels; however, only 48 of them are actually required for the 2x2. Each TPC has 4 light collection module with 6 sipms each --> 24 channels per TPC --> 48 channels per module. But the hardware has enough for 96 channels each. |
@marjoleinvannuland I'm completely screwing this up... sorry for the confusion. You are correct, we should indeed have 384 active channels. |
@marjoleinvannuland for your test file where you printed |
I checked an entire file. Even if not all channels are there, I do not expect exactly half of them to be missing. But if I add the indent, I get all the channels back. Also less segments seem to have multiple event_ids. However, some of them still have. So, it does not fix everything.
|
@marjoleinvannuland Should we get this fix into develop? We noticed some undesirable features in the MiniRun5_beta1 files that I suspect are connected to this. E.g. see page 13 of https://portal.nersc.gov/project/dune/data/2x2/simulation/productions/MiniRun5_1E19_RHC/MiniRun5_1E19_RHC.plots.beta1/PLOTS/LARNDSIM/0000000/MiniRun5_1E19_RHC.larnd.0000012.LARNDSIM_validations.pdf, also included here for convenience: |
larnd-sim/cli/simulate_pixels.py Line 976 in 70d3391
selected_tracks['segment_id'] solves the problem with the segments that correspond to multiple triggers and event_ids. I am still going to do some checks and validation on the output (@krwood @YifanC @russellphysics)Edit: larndsim validation |
@marjoleinvannuland Cool! I think this looks like it! |
If we need to use the index of the segment_id instead of the segment_id itself, I think this line should be outside the loop over the modules. I see no reason to not use the segment_id, but if we plan to make them not unique in a file in the foreseeable future maybe we should consider this? |
FYI, since it was only briefly mentioned here, I opened issue #223 where I explain what I observe regarding the missing light channels |
Currently the light backtracking information is stored in
[('trigger_id', '<i4'), ('op_channel_id', '<i4'), ('tick', '<i4'), ('event_id', '<i4'), ('segment_id', '<i8'), ('pe_current', '<f8')]
. The change is made in this PR.Quinn @qhvuong noticed that a single segment can correspond to multiple triggers in the current output (985413e). Tested on
MiniRun5_1E19_RHC.convert2h5.00666.EDEPSIM.hdf5
. See the snapshot of some examples:The rate of such occurrence is very high. The true
event_id
correspond to a segment is not necessary in the list ofevent_id
s. For example segment 4507 is actually from event 666019.Tag @marjoleinvannuland @russellphysics @qhvuong @krwood @mjkramer to follow up on this issue. For the moment, we plan to carry on MR5 production, and implement the fix for this issue in MR5.1.
The text was updated successfully, but these errors were encountered: