-
Notifications
You must be signed in to change notification settings - Fork 7
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
RmsFiller tools missing in vertical-drift reco #22
Comments
Looking in dunedataprep/DataPrep/fcl at vdcb2_tools.fcl and vdcb_dataprep_sequences, I do the tool vdcb_adcChannelRawRmsFiller is defined but the sequences use vdbcb_adcChannelRawRmsFiller and vdtcb_adcChannelRawRmsFiller. I modified the sequence to instead use the first tool and dropped a line which had no effect in the tools file. |
The fix is committed. Wenqiang, can you check that all looks OK if you work with the head of this package (dunedataprep) or in the next release? We should check both crp2 and crp3 reco. Thank you. |
BTW, I checked and the crp2 and crp3 configs only differ in channel status: duneproc> fcldump crp2cb_data_jul2022_reco.fcl 5 >crp2cb_data_jul2022_reco.fcldump duneproc> fcldump crp3cb_data_oct2022_reco.fcl 5 >crp3cb_data_oct2022_reco.fcldump duneproc> l crp*dump 193 -rw-r--r--. 1 dladams fnalgrid 197040 Mar 2 14:22 crp2cb_data_jul2022_reco.fcldump 193 -rw-r--r--. 1 dladams fnalgrid 196998 Mar 2 14:23 crp3cb_data_oct2022_reco.fcldump duneproc> diff crp*dump 1c1 < /home/dladams/proc/build/dev01/workdir/localProducts_dunesw_v09_68_00d00_e20_prof/dunesw/v09_68_00d00/fcl/crp2cb_data_jul2022_reco.fcl --- > /home/dladams/proc/build/dev01/workdir/localProducts_dunesw_v09_68_00d00_e20_prof/dunesw/v09_68_00d00/fcl/crp3cb_data_oct2022_reco.fcl 8c8 < process_name: "CRP2CBDataReco" --- > process_name: "CRP3CBDataReco" 1635c1635 < NoisyChannels: [263, 264, 265, 266, 1297, 1298, 2667, 2668] --- > NoisyChannels: [] duneproc> |
We should close this as soon as we have positive feedback here or when dunesw issue 53 is closed. |
Hi @dladams , the new release v09_69_00d00 has included this fixing. However, we noticed that there is always a "Segmentation fault" at the end of a reco job for coldbox data. Since v09_69_00d00 has some other commits, I have again tested based on v09_68_00d00 and with a minimal change to "vdcb_dataprep_sequences.fcl". I can reproduce the "segmentaion fault". Could you take a look at this issue? Could it be a memory issue? Here is my command:
|
I am looking into this. --david |
I also see a crash. It occurs in C++ cleanup in the AdcChannelStatus dtor when the channel status service is accessed through a bare pointer. Presumably the latter object has already been destroyed and I laid the trap for myself with this code in the ctor: m_pChannelStatusProvider = &art::ServiceHandle()->GetProvider(); I will try holding the service handle instead of (or in addition to) the bare pointer. I have added Kyle who may be able to confirm this is the right approach or suggest another. |
@dladams, can you send me a link to the code in question? |
The code is here but, after thinking a bit, I realize the channel status service should not be used during cleanup since we are not inside of an event. A crash isn't a very nice way to advertise that but good to know the code was doing something wrong. I fixed the problem by zeroing the pointer in the dtor. I no longer get the crash. |
The fix is pushed. Please test and report back here but I am confident the problem has resolved. I will also have a peak at the config. We probably don't want to be making any plots much less summary plots in this context. |
The tool was originally written to make plots of metric (here RMS) vs. channel and makes the graphs even if not configured to print to write out the graphs. The feature to write the metric to the (transient) event metadata was added later. That is the feature we need here. Other that wasting a few cycles and little memory, there should be no harm in using the code as it is. If you don't want update to use the new code (now pushed to dunedataprep develop), the crash can also be avoided and the number of graphs reduced by a factor four by setting tools.vdcb_adcChannelRawRmsFiller.PlotUsesStatus: 0 |
Ah, I see. Services are destroyed after the art source and art modules...which means the tools are probably owned by a statically allocated object somewhere. Yes, that's fragile. |
Wenqiang report that a tool is missing in vertical-drift reco:
The text was updated successfully, but these errors were encountered: