-
Notifications
You must be signed in to change notification settings - Fork 35
Feature/maxd bnb qual cut #551
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
Conversation
|
Added Gianluca to the list of reviewers, as a CAF expert |
PetrilloAtWork
left a comment
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.
The code here is tough, in many places obscure with references lost in time.
I have left comments throughout it, with the general intention that:
- code be documented in Doxygen format
- at least the parts that are more under our control (e.g. quality selections and cut) be documented and shaped in a maintainable way
- code be adjusted to be more readable and maintainable
- a likely memory leak (involving, unsurprisingly, ROOT) be addressed.
Thank you for taking this hard task on your shoulders!
sbncode/BeamSpillInfoRetriever/SBNDBNBRetriever/SBNDBNBRetriever_module.cc
Outdated
Show resolved
Hide resolved
sbncode/BeamSpillInfoRetriever/SBNDBNBRetriever/SBNDBNBRetriever_module.cc
Outdated
Show resolved
Hide resolved
sbncode/BeamSpillInfoRetriever/SBNDBNBRetriever/SBNDBNBRetriever_module.cc
Outdated
Show resolved
Hide resolved
sbncode/BeamSpillInfoRetriever/SBNDBNBRetriever/SBNDBNBRetriever_module.cc
Outdated
Show resolved
Hide resolved
…er_module.cc Co-authored-by: Gianluca Petrillo <[email protected]>
…er_module.cc Co-authored-by: Gianluca Petrillo <[email protected]>
…er_module.cc Co-authored-by: Gianluca Petrillo <[email protected]>
|
✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard |
|
✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for c14: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 |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for c14: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 |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for e26: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 |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for e26: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 |
|
@maxdubnowski , I am seeing failures in If you could take a look at this when you can, it would be much appreciated. Thanks! |
|
trigger build ci_ref=v10_06_02 LArSoft/lar*@LARSOFT_SUITE_v10_09_00 SBNSoftware/sbncode@v10_09_00 SBNSoftware/sbnalg@v10_09_00 SBNSoftware/sbnobj#135 SBNSoftware/sbnanaobj#146 SBNSoftware/sbndaq_artdaq_core@v1_10_06 SBNSoftware/sbndata@v01_07 SBNSoftware/sbndutil@v10_06_00_02 SBNSoftware/sbnd_data@v01_34_00 |
|
✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for e26: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 |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for e26: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 |
|
✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for c14: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 |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for c14: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 |
|
trigger build ci_ref=v10_06_02 LArSoft/lar*@LARSOFT_SUITE_v10_09_00 SBNSoftware/sbnobj#135 SBNSoftware/sbnanaobj#146 |
|
✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard |
|
✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for c14: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 |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for c14: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 |
|
🚨 For more details about the warning phase, check the ci_tests SBND phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase ci_tests ICARUS on slf7 for e26:prof - ignored warnings for build -- details available through the CI dashboard 🚨 For more details about the failed phase, check the ci_tests ICARUS phase logs parent CI build details are available through the CI dashboard |
jacoblarkin
left a comment
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.
Looks good from my side and doesn't seem to affect anything ICARUS related. At some point we should test this implementation of the FOM with ICARUS as well, but that's not necessary to approve this.
Description
Added additional information to the BNBSpillInfo module including additional monitors and monitor offsets from IFBeam that allow for the creation of the BNB FOM adapted from MicroBooNE using sbnd information. My DocDB presentation: https://sbn-docdb.fnal.gov/cgi-bin/sso/ShowDocument?docid=42480
I am not sure who is a CAF maintainer...
Requires linking sbnobj and sbnanaobj. Their PRs are listed below:
sbnobj: PR135 SBNSoftware/sbnobj#135
sbnanaobj: PR146 SBNSoftware/sbnanaobj#146