Skip to content

Conversation

@jzennamo
Copy link
Contributor

This requires two things:

  1. sbnobj, feature/zennamo_bnb_spill_accounting
  2. larsoft, v09_24_02 or greater (specifically ifbeam v2_5_0)

This enables us to store, onto the art::SubRun, the beam spill information based on the times from the artdaq::Fragments.

It does not currently support:

  1. store information from the MultiWire Chambers
  2. correctly handle the first event in a Run
    These will be added as a "next step".

@jzennamo jzennamo requested a review from PetrilloAtWork June 17, 2021 19:27
@cjbacchus
Copy link
Contributor

Very excited to see this. Is accumulating all the records in the subrun a conscious decision? How about putting the records for all spills between one event and the next into that second event? Or just the record for that particular spill, and all the others still wind up in the subrun? Or some combination of these strategies?

I'm thinking that the current setup wouldn't allow CAFMaker to access this info until endSubRun() if you ran the two modules in the same job. Maybe we're stuck with that anyway because of spills after the last trigger? And the CAFs are going to have to wind up storing this information in a different tree? From experience, correlating that with the main tree is a total pain if you're trying to apply any spill quality cuts.

I was actually hoping we could have a system where you could run over any arbitrary number of events in CAF and get the right exposure, rather than being forced to treat them by subrun.

@jzennamo
Copy link
Contributor Author

Hi @cjbackhouse, thanks for the feedback! I'll respond to the points one at a time:

Is accumulating all the records in the subrun a conscious decision?

It is! We can filter events and if we do we would throw away this information. By storing it in the SubRun it will persist even if the file is filtered. One could imagine writing the triggered spill to the art::Event for matching purposes, I can look into that, but one should be able to match based on the time of the event.

I'm thinking that the current setup wouldn't allow CAFMaker to access this info until endSubRun() if you ran the two modules in the same job.

I believe you are correct, but this module is intended to be run as part of Stage0/reco1 processing, meaning that it wouldn't be run with CAF files in the same pass.

I was actually hoping we could have a system where you could run over any arbitrary number of events in CAF and get the right exposure, rather than being forced to treat them by subrun.

It is unclear to me how this would be possible, would it be by saving the POT to the art::Event? What about if you remove art::Events via, say, optical filtering, I think your art::Events would then return a biased POT count, right?

@cjbacchus
Copy link
Contributor

One could imagine writing the triggered spill to the art::Event for matching purposes

I think this would be useful

It is unclear to me how this would be possible, would it be by saving the POT to the art::Event? What about if you remove art::Events via, say, optical filtering, I think your art::Events would then return a biased POT count, right?

You'd need a scheme by which the cut event's POT was rescued and associated with the next event that survived. I agree this sounds very fiddly.

The other design would be to have every Event continue to exist as some "trace", where the basic details about the trigger and so on continue to exist, and some flag explaining why it was filtered out, but then all the other products are dropped.

@jzennamo
Copy link
Contributor Author

@PetrilloAtWork I moved this forward to a SBNCode release that supports the changes to ifbeam I have also added the functionality to match the multiwire devices to the beam spills

@jzennamo
Copy link
Contributor Author

@PetrilloAtWork I have not added the correct handling for the first event in a run. This does not currently work in ICARUS because the gate counter does not discriminate on the trigger stream. This will be eventually added so I added it to the code here.

@jzennamo
Copy link
Contributor Author

@PetrilloAtWork I added a (trivial) module that will collect the number of external triggers. I think this is my last development on this and I do not expect to change my code any further

Copy link
Member

@PetrilloAtWork PetrilloAtWork left a comment

Choose a reason for hiding this comment

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

I have generously decorated the code with technical comments, which may be addressed as you deem proper (after all, this is still a proof of concept).
It was discussed already that a trigger data product is in development for ICARUS (SBN) that will apparently include all the information the extractors need, saving us from doing decoding a third time (trigger and PMT decoding do already, and the latter should also stop).
I have hard time following the intricacies of wire chamber data decryption, so don't rely on my review to catch issues in there.
I would like to recommend that the event number be assigned to all the spills extracted around it (unsigned int sbn::BNBSpillInfo::event;) for easy backtracking, and that the one which is considered to be the spill for that event be marked as such (bool sbn::BNBSpillInfo::hasEvent { false };). It is a bit of a waste, but it's not my wallet.
Also we should document clearly what resolution we can expect for these times. It surely can't go significantly better than the microsecond.
Considering the time matching algorithm and the potentially missing data, it is not unlikely that the same hardware record is matched for multiple times. I don't know if this is acceptable, if it is a problem and if it should be monitored.

Finally, out of all of this I still have no idea how to count POT. If there is an obvious, trivial way, I would add its code in the data product (double sbn::BNBSpillInfo::POT() const); if there is an obvious, more complicate way, I would add its result to the data product (double sbn::BNBSpillInfo::POT;).

@PetrilloAtWork
Copy link
Member

PetrilloAtWork commented Jun 26, 2021

I was actually hoping we could have a system where you could run over any arbitrary number of events in CAF and get the right exposure, rather than being forced to treat them by subrun.

@cjbackhouse: can you precisely describe the use case you envision?
For example, I can't understand "run over any arbitrary number of events in CAF and get the right exposure"... while I would understand "run over any arbitrary time interval and get the right exposure".
While I have the feeling of what you mean, and of that being extremely valuable, when I try to put the finger on it, it vanishes.

@cjbacchus
Copy link
Contributor

cjbacchus commented Jun 28, 2021

I was actually hoping we could have a system where you could run over any arbitrary number of events in CAF and get the right exposure, rather than being forced to treat them by subrun.

@cjbackhouse: can you precisely describe the use case you envision?
For example, I can't understand "run over any arbitrary number of events in CAF and get the right exposure"... while I would understand "run over any arbitrary time interval and get the right exposure".
While I have the feeling of what you mean, and of that being extremely valuable, when I try to put the finger on it, it vanishes.

I'm coming from NOvA where every beam spill causes a trigger (ie readout) in the detector, and we keep all of those in the data files, even though most of them are empty (they had plenty of cosmics to begin with, but they all failed early selection stages). There, if you loop over any arbitrary number of events (not even complete files) you can just sum the POT associated with each event.

In SBN, I think I'm assuming that each event implicitly includes the untriggered spills before it. Then you could sum the "this spill and all earlier untriggered spills POT" number for each CAF record you use and get correct POT (modulo odd questions about the bias introduced by this definition of the time range you use. I think the weirdness of the events before the first event actually cancels with the weirdness of the missing time after the last event). This can work with the first event in the file/subrun (all missing triggers since the subrun began) but not with the "acausal" triggers after the last event. But I think that problem isn't even understood at the ART level and Joseph is punting on it for now.

@FNALbuild
Copy link

⚠️ CI build for ICARUS Warning at phase ci_tests ICARUS on slf7 for e20:prof - ignored warnings for build -- details available through the CI dashboard

🚨 For more details about the warning phase, check the ci_tests ICARUS phase logs

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link

CI build for LArSoft on slf7 for c7:prof is in progress -- details available through the CI dashboard

@FNALbuild
Copy link

CI build for LArSoft on slf7 for e20:prof is in progress -- details available through the CI dashboard

@FNALbuild
Copy link

✔️ CI build for LArSoft Succeeded on slf7 for c7:prof -- details available through the CI dashboard

@FNALbuild
Copy link

✔️ CI build for LArSoft Succeeded on slf7 for e20:prof -- details available through the CI dashboard

@FNALbuild
Copy link

CI build for SBND on slf7 for c7:prof is in progress -- details available through the CI dashboard

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link

CI build for ICARUS on slf7 for c7:prof is in progress -- details available through the CI dashboard

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link

CI build for ICARUS on slf7 for e20:prof is in progress -- details available through the CI dashboard

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link

CI build for SBND on slf7 for e20:prof is in progress -- details available through the CI dashboard

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link

❌ CI build for SBND Failed at phase build SBND on slf7 for c7:prof -- details available through the CI dashboard

🚨 For more details about the failed phase, check the build SBND phase logs

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link

❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for c7:prof -- details available through the CI dashboard

🚨 For more details about the failed phase, check the build ICARUS phase logs

parent CI build details are available through the CI dashboard

@jzennamo
Copy link
Contributor Author

OK, the c7 builds failed for this:

Does anyone have any thoughts?

1066: FAILED: sbncode/sbncode/BeamSpillInfoRetriever/BNBRetriever/CMakeFiles/sbn_BNBSpillInfoRetriever_MWRData.dir/MWRData.cpp.o 
1067: /cvmfs/larsoft.opensciencegrid.org/products/clang/v7_0_0/Linux64bit+3.10-2.17/bin/clang++ -DNDEBUG -Dsbn_BNBSpillInfoRetriever_MWRData_EXPORTS -I/scratch/workspace/sbnd_ci/label_exp/SLF7/label_exp2/swarm/SBND/srcs/sbnanaobj -I/scratch/workspace/sbnd_ci/label_exp/SLF7/label_exp2/swarm/SBND/srcs/sbndaq_artdaq_core -I/scratch/workspace/sbnd_ci/label_exp/SLF7/label_exp2/swarm/SBND/srcs/sbnobj -Isbnobj -Isbndaq_artdaq_core -Isbnanaobj -I/scratch/workspace/sbnd_ci/label_exp/SLF7/label_exp2/swarm/SBND/srcs/sbncode -Isbncode -I/scratch/workspace/sbnd_ci/label_exp/SLF7/label_exp2/swarm/SBND/srcs/sbndutil -Isbndutil -I/scratch/workspace/sbnd_ci/label_exp/SLF7/label_exp2/swarm/SBND/srcs/sbndcode -Isbndcode -I/cvmfs/larsoft.opensciencegrid.org/products/art/v3_09_03/include -I/cvmfs/larsoft.opensciencegrid.org/products/messagefacility/v2_08_04/include -I/cvmfs/larsoft.opensciencegrid.org/products/art_root_io/v1_08_03/include -I/cvmfs/larsoft.opensciencegrid.org/products/gsl/v2_6a/Linux64bit+3.10-2.17/include -I/cvmfs/larsoft.opensciencegrid.org/products/nusimdata/v1_24_05/include -I/cvmfs/larsoft.opensciencegrid.org/products/clhep/v2_4_4_1/Linux64bit+3.10-2.17-c7-prof/lib/CLHEP-2.4.4.1/../../include -I/cvmfs/larsoft.opensciencegrid.org/products/nug4/v1_11_00/include -I/cvmfs/larsoft.opensciencegrid.org/products/nugen/v1_14_05/include -I/cvmfs/larsoft.opensciencegrid.org/products/nurandom/v1_07_06/include -I/cvmfs/larsoft.opensciencegrid.org/products/nutools/v3_11_05/include -I/cvmfs/larsoft.opensciencegrid.org/products/genie/v3_00_06p/Linux64bit+3.10-2.17-c7-prof/include -I/cvmfs/larsoft-ib.opensciencegrid.org/LArSoft/LArSoft_slf7_c7_prof_20211014_023013/localProducts_LArSoft_LArSoft_lar_ci_c7_prof/lardataobj/v09_03_02/include -I/cvmfs/larsoft-ib.opensciencegrid.org/LArSoft/LArSoft_slf7_c7_prof_20211014_023013/localProducts_LArSoft_LArSoft_lar_ci_c7_prof/larcoreobj/v09_03_00/include -I/cvmfs/larsoft-ib.opensciencegrid.org/LArSoft/LArSoft_slf7_c7_prof_20211014_023013/localProducts_LArSoft_LArSoft_lar_ci_c7_prof/larcorealg/v09_03_00/include -I/cvmfs/larsoft-ib.opensciencegrid.org/LArSoft/LArSoft_slf7_c7_prof_20211014_023013/localProducts_LArSoft_LArSoft_lar_ci_c7_prof/lardataalg/v09_06_02/include -I/cvmfs/larsoft-ib.opensciencegrid.org/LArSoft/LArSoft_slf7_c7_prof_20211014_023013/localProducts_LArSoft_LArSoft_lar_ci_c7_prof/larcore/v09_03_00/include -I/cvmfs/larsoft-ib.opensciencegrid.org/LArSoft/LArSoft_slf7_c7_prof_20211014_023013/localProducts_LArSoft_LArSoft_lar_ci_c7_prof/lardata/v09_03_02/include -I/cvmfs/larsoft-ib.opensciencegrid.org/LArSoft/LArSoft_slf7_c7_prof_20211014_023013/localProducts_LArSoft_LArSoft_lar_ci_c7_prof/larevt/v09_03_02/include -I/cvmfs/larsoft-ib.opensciencegrid.org/LArSoft/LArSoft_slf7_c7_prof_20211014_023013/localProducts_LArSoft_LArSoft_lar_ci_c7_prof/larsim/v09_17_00/include -I/cvmfs/larsoft-ib.opensciencegrid.org/LArSoft/LArSoft_slf7_c7_prof_20211014_023013/localProducts_LArSoft_LArSoft_lar_ci_c7_prof/larreco/v09_07_03/include -I/cvmfs/larsoft-ib.opensciencegrid.org/LArSoft/LArSoft_slf7_c7_prof_20211014_023013/localProducts_LArSoft_LArSoft_lar_ci_c7_prof/larpandora/v09_09_03/include -I/cvmfs/larsoft.opensciencegrid.org/products/TRACE/v3_17_01/include -I/cvmfs/larsoft.opensciencegrid.org/products/artdaq_core/v3_07_04/include -I/cvmfs/larsoft.opensciencegrid.org/products/libwda/v2_29_1/Linux64bit+3.10-2.17/include -I/cvmfs/larsoft.opensciencegrid.org/products/ifdhc/v2_5_16/Linux64bit+3.10-2.17-c7-p392-prof/inc -I/cvmfs/larsoft.opensciencegrid.org/products/ifdh_art/v2_12_05/slf7.x86_64.c7.prof/inc -I/cvmfs/larsoft.opensciencegrid.org/products/log4cpp/v1_1_3c/Linux64bit+3.10-2.17-c7-prof/include -I/cvmfs/larsoft.opensciencegrid.org/products/root/v6_22_08d/Linux64bit+3.10-2.17-c7-p392-prof/include -I/cvmfs/larsoft.opensciencegrid.org/products/ifbeam/v2_5_2/Linux64bit+3.10-2.17-c7-p392-prof/include -g -O3 -fno-omit-frame-pointer -DNDEBUG -fPIC -Werror -Wno-error=deprecated-declarations -grecord-gcc-switches -Wall -Werror=return-type -pedantic -Wno-unused-local-typedefs -Wno-undefined-var-template -std=c++17 -MD -MT sbncode/sbncode/BeamSpillInfoRetriever/BNBRetriever/CMakeFiles/sbn_BNBSpillInfoRetriever_MWRData.dir/MWRData.cpp.o -MF sbncode/sbncode/BeamSpillInfoRetriever/BNBRetriever/CMakeFiles/sbn_BNBSpillInfoRetriever_MWRData.dir/MWRData.cpp.o.d -o sbncode/sbncode/BeamSpillInfoRetriever/BNBRetriever/CMakeFiles/sbn_BNBSpillInfoRetriever_MWRData.dir/MWRData.cpp.o -c /scratch/workspace/sbnd_ci/label_exp/SLF7/label_exp2/swarm/SBND/srcs/sbncode/sbncode/BeamSpillInfoRetriever/BNBRetriever/MWRData.cpp
1068: In file included from /scratch/workspace/sbnd_ci/label_exp/SLF7/label_exp2/swarm/SBND/srcs/sbncode/sbncode/BeamSpillInfoRetriever/BNBRetriever/MWRData.cpp:6:
1069: In file included from /usr/include/boost/algorithm/string.hpp:23:
1070: In file included from /usr/include/boost/algorithm/string/split.hpp:16:
1071: In file included from /usr/include/boost/algorithm/string/iter_find.hpp:27:
1072: In file included from /usr/include/boost/algorithm/string/find_iterator.hpp:24:
1073: In file included from /usr/include/boost/algorithm/string/detail/find_iterator.hpp:18:
1074: In file included from /usr/include/boost/function.hpp:24:
1075: In file included from /usr/include/boost/function/detail/prologue.hpp:18:
1076: In file included from /usr/include/boost/mem_fn.hpp:22:
1077: In file included from /usr/include/boost/bind/mem_fn.hpp:25:
1078: /usr/include/boost/get_pointer.hpp:27:23: error: redefinition of 'get_pointer' as different kind of symbol
1079: template<class T> T * get_pointer(std::auto_ptr<T> const& p)
1080:                       ^
1081: /usr/include/boost/get_pointer.hpp:20:23: note: previous definition is here
1082: template<class T> T * get_pointer(T * p)
1083:                       ^
1084: /usr/include/boost/get_pointer.hpp:27:40: error: no member named 'auto_ptr' in namespace 'std'
1085: template<class T> T * get_pointer(std::auto_ptr<T> const& p)
1086:                                   ~~~~~^
1087: /usr/include/boost/get_pointer.hpp:27:49: error: 'T' does not refer to a value
1088: template<class T> T * get_pointer(std::auto_ptr<T> const& p)
1089:                                                 ^
1090: /usr/include/boost/get_pointer.hpp:27:16: note: declared here
1091: template<class T> T * get_pointer(std::auto_ptr<T> const& p)
1092:                ^
1093: /usr/include/boost/get_pointer.hpp:27:52: error: expected expression
1094: template<class T> T * get_pointer(std::auto_ptr<T> const& p)
1095:                                                    ^
1096: /usr/include/boost/get_pointer.hpp:27:61: error: expected ';' at end of declaration
1097: template<class T> T * get_pointer(std::auto_ptr<T> const& p)
1098:                                                             ^
1099: /usr/include/boost/get_pointer.hpp:28:1: error: expected unqualified-id
1100: {
1101: ^
1102: 6 errors generated.

@FNALbuild
Copy link

⚠️ CI build for SBND Succeeded with warning at phase build on slf7 for e20:prof - ignored warnings for build -- details available through the CI dashboard

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link

⚠️ CI build for ICARUS Warning at phase ci_tests ICARUS on slf7 for e20:prof - ignored warnings for build -- details available through the CI dashboard

🚨 For more details about the warning phase, check the ci_tests ICARUS phase logs

parent CI build details are available through the CI dashboard

@cjbacchus
Copy link
Contributor

cjbacchus commented Oct 15, 2021

@jzennamo /usr/include/boost/... doesn't look right. You should fiddle with your makefiles until you're picking up the proper ups version of boost.

@FNALbuild
Copy link

CI build for LArSoft on slf7 for c7:prof is in progress -- details available through the CI dashboard

@FNALbuild
Copy link

✔️ CI build for LArSoft Succeeded on slf7 for c7:prof -- details available through the CI dashboard

@FNALbuild
Copy link

CI build for SBND on slf7 for c7:prof is in progress -- details available through the CI dashboard

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link

CI build for ICARUS on slf7 for c7:prof is in progress -- details available through the CI dashboard

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link

CI build for LArSoft on slf7 for e20:prof is in progress -- details available through the CI dashboard

@FNALbuild
Copy link

✔️ CI build for LArSoft Succeeded on slf7 for e20:prof -- details available through the CI dashboard

@FNALbuild
Copy link

CI build for SBND on slf7 for e20:prof is in progress -- details available through the CI dashboard

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link

CI build for ICARUS on slf7 for e20:prof is in progress -- details available through the CI dashboard

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link

⚠️ CI build for SBND Warning at phase ci_tests SBND on slf7 for c7:prof - ignored warnings for build -- details available through the CI dashboard

🚨 For more details about the warning phase, check the ci_tests SBND phase logs

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link

⚠️ CI build for SBND Warning at phase ci_tests SBND on slf7 for e20:prof - ignored warnings for build -- details available through the CI dashboard

🚨 For more details about the warning phase, check the ci_tests SBND phase logs

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link

⚠️ CI build for ICARUS Warning at phase ci_tests ICARUS on slf7 for c7:prof - ignored warnings for build -- details available through the CI dashboard

🚨 For more details about the warning phase, check the ci_tests ICARUS phase logs

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link

⚠️ CI build for ICARUS Warning at phase ci_tests ICARUS on slf7 for e20:prof - ignored warnings for build -- details available through the CI dashboard

🚨 For more details about the warning phase, check the ci_tests ICARUS phase logs

parent CI build details are available through the CI dashboard

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

None yet

Development

Successfully merging this pull request may close these issues.

8 participants