Feature/icarus bnb only #523

nathanielerowe · 2025-03-19T20:39:53Z

Fixes #479, restructures code so SBND BNB modules and ICARUS BNB modules can share some code. Open to feedback on the best way to handle this, since I did not write the original BNBRetriever module.

Updates from develop

catch up w/ develop

nathanielerowe · 2025-04-03T15:26:15Z

Might need an additional PR to account for changes in the icarus pot label, still in the process of figuring out where that would be stored.

PetrilloAtWork

I wonder the sense of leaving modules that are now plainly experiment-specific in sbncode.

Comments on POTTools.h:

Thank you for extracting the common code, it's very useful and I hope will save time in the future.
Could you move the description of the functions from the comments in the implementation file to the header in Doxygen format?
I feel these functions may be better enclosed in their own namespace. In particular, a class named TriggerInfo_t can contain many different things, and I am not sure if we want to commit the sbn::TriggerInfo_t name to this one (I can count at least three TriggerInfo_t in the ICARUS code stack, two are class members and one is icarus::trigger::details::TriggerInfo_t); proposal: sbn::pot (module files are allowed to using namespace sbn::pot;, although I still recommend against it).

PetrilloAtWork · 2025-04-03T18:12:49Z

sbncode/BeamSpillInfoRetriever/job/icarusbnbspillinfo.fcl

    TimePadding: 0.0333 #unit seconds, Booster Rep Rate is 15 Hz, so the closest spill could be 66ms away
+    BESOffset: 0.036 #unit seconds, we expect the BES to be about 35 ms from $1D. Need to adjust search range accordingly.


✔️ From some tests it appears to be about 35.746 ms in ICARUS.

This is set now!

(now you are going to kill me...)
My intention was to confirm the value that you inserted. On one side, 1 ms does not matter since when we match to the database we have elements 66 ms apart. On the other, if you put a number with four significant digits, that begs a reference, possibly something more reliable than "Gianluca wrote so in a chat"; I have not presented that value publicly yet, and I don't know when I will (and if then I will be told I did it all wrong).
In fact, also the previous 0.036 begs for a reference, just less loudly.
If nothing better, you can cite SBN DocDB 40669. Which does not exist, but I promise it will at a certain point in the future.

No worries, references are always better to have than not imo, so I will put that link in when I get a chance.

sbncode/BeamSpillInfoRetriever/ICARUSBNBRetriever/ICARUSBNBRetriever_module.cc

PetrilloAtWork

I have left one more comment that the author can take action upon, but I consider the PR approved.

kjplows · 2025-04-25T22:53:24Z

trigger build

FNALbuild · 2025-04-25T22:56:19Z

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

FNALbuild · 2025-04-25T22:56:19Z

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

FNALbuild · 2025-04-25T22:57:20Z

❌ 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

FNALbuild · 2025-04-25T22:57:22Z

❌ 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

FNALbuild · 2025-04-25T23:12:51Z

⚠️ CI build for ICARUS Warning at phase ci_tests ICARUS on slf7 for e26:prof -- 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 · 2025-04-25T23:20:16Z

⚠️ CI build for SBND Warning at phase ci_tests SBND on slf7 for e26:prof -- 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

catch up

PetrilloAtWork · 2025-05-06T22:35:21Z

sbncode/BeamSpillInfoRetriever/job/run_icarusbnbinfo_sbn.fcl


-process_name: BNBInfoGen
+process_name: ICARUSBNBInfoGen


Alternative proposal following the suggestion from SBNSoftware/icaruscode#823:

Suggested change

process_name: ICARUSBNBInfoGen

process_name: BNBInfoGen

PetrilloAtWork · 2025-05-06T22:35:40Z

sbncode/BeamSpillInfoRetriever/job/run_icarusbnbinfo_sbn.fcl

@@ -28,17 +28,17 @@ source: {

 physics: {
 producers: {
-   bnbinfo: @local::bnbspillinfo
+   icarusbnbinfo: @local::icarusbnbspillinfo


Alternative proposal following the suggestion from SBNSoftware/icaruscode#823:

Suggested change

icarusbnbinfo: @local::icarusbnbspillinfo

bnbinfo: @local::icarusbnbspillinfo

PetrilloAtWork · 2025-05-06T22:35:56Z

sbncode/BeamSpillInfoRetriever/job/run_icarusbnbinfo_sbn.fcl

 }

- simulate: [bnbinfo ]
+ simulate: [icarusbnbinfo ]


Alternative proposal following the suggestion from SBNSoftware/icaruscode#823:

Suggested change

simulate: [icarusbnbinfo ]

simulate: [ bnbinfo ]

PetrilloAtWork · 2025-05-06T22:36:50Z

sbncode/BeamSpillInfoRetriever/job/run_icarusbnbinfo_sbn.fcl

 stream1: [ out1 ]
 }

 outputs: {
 out1: {
   module_type: RootOutput
-   fileName: "%ifb_%tc_bnbinfo.root"
+   fileName: "%ifb_%tc_icarusbnbinfo.root"


Alternative proposal following the suggestion from SBNSoftware/icaruscode#823:

Suggested change

fileName: "%ifb_%tc_icarusbnbinfo.root"

fileName: "%ifb_%tc_bnbinfo.root"

(the change in this PR could actually affecting the workflow by changing the output file names)

Nathaniel Rowe and others added 28 commits October 9, 2024 14:50

initial commit

1df3234

Develop change not in release 09_91_02_02

357c6e5

Builds and runs

b205ea9

debug and cache changes

225eaf9

Fill cache

030248d

Get timestamp from PTB

076033d

Refactor PTB ts extraction

8b5216d

Add TDC Timestamp func

574348a

Add prev ptb timestamp

fc0776a

Cleaning print statements

c5aaeb9

Undo build fix, get rid of junk

f528857

Moving fcls to jobs

8a7fbf8

Rename ICARUS module

12b93b4

Build issues returned to match branch

3742907

Rename icarusspillinfo.fcl to icarusbnbspillinfo.fcl

1656232

New producer modules

6e3023f

Rename EXT producers, add catch for TDC

e2d8f6a

Temp debug lines, TDC catch

96682e9

remove scaling, add debug

83f8895

Merge branch 'develop' into feature/SBNDBNB_Retriever

de192fe

Merge pull request #505 from SBNSoftware/develop

749e031

Updates from develop

Remove SBND modules

448561a

$1D offset was hardcoded! Change BES offset to fcl param.

0aef7af

Merge pull request #520 from SBNSoftware/develop

00c4192

catch up w/ develop

Merge branch 'develop' into feature/icarus_bnb_only

34c8218

remove newly-duplicate code in icarus

f1a7e63

rename POTTools

bbb400f

matchMultiWire changes

631dc9f

nathanielerowe added maintenance bugfix Addresses one or more bugs labels Mar 19, 2025

nathanielerowe requested review from jzennamo, SFBayLaser and PetrilloAtWork March 19, 2025 20:39

nathanielerowe self-assigned this Mar 19, 2025

kjplows added this to SBN software development Mar 20, 2025

kjplows moved this to PR in progress in SBN software development Mar 20, 2025

nathanielerowe and others added 2 commits April 3, 2025 09:45

Merge branch 'develop' into feature/icarus_bnb_only

858a129

Remove extra fcl

f5403aa

nathanielerowe marked this pull request as ready for review April 3, 2025 15:25

PetrilloAtWork reviewed Apr 3, 2025

View reviewed changes

Doxygen+namespace updates

1e60e40

PetrilloAtWork approved these changes Apr 8, 2025

View reviewed changes

kjplows moved this from PR in progress to Partially reviewed in SBN software development Apr 10, 2025

Merge branch 'develop' into feature/icarus_bnb_only

0d1282e

Merge pull request #539 from SBNSoftware/develop

d9cdf7d

catch up

nathanielerowe mentioned this pull request May 4, 2025

Update cafmakerjob_icarus_data.fcl SBNSoftware/icaruscode#823

Open

PetrilloAtWork reviewed May 6, 2025

View reviewed changes

		TimePadding: 0.0333 #unit seconds, Booster Rep Rate is 15 Hz, so the closest spill could be 66ms away
		BESOffset: 0.036 #unit seconds, we expect the BES to be about 35 ms from $1D. Need to adjust search range accordingly.

	icarusbnbinfo: @local::icarusbnbspillinfo
	bnbinfo: @local::icarusbnbspillinfo

	fileName: "%ifb_%tc_icarusbnbinfo.root"
	fileName: "%ifb_%tc_bnbinfo.root"

Feature/icarus bnb only #523

Are you sure you want to change the base?

Feature/icarus bnb only #523

Uh oh!

Conversation

nathanielerowe commented Mar 19, 2025

Uh oh!

nathanielerowe commented Apr 3, 2025

Uh oh!

PetrilloAtWork left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PetrilloAtWork left a comment

Choose a reason for hiding this comment

Uh oh!

kjplows commented Apr 25, 2025

Uh oh!

FNALbuild commented Apr 25, 2025

Uh oh!

FNALbuild commented Apr 25, 2025

Uh oh!

FNALbuild commented Apr 25, 2025

Uh oh!

FNALbuild commented Apr 25, 2025

Uh oh!

FNALbuild commented Apr 25, 2025

Uh oh!

FNALbuild commented Apr 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!