Adding offbeam gate counting into the CAFs by jzettle · Pull Request #298 · SBNSoftware/sbncode

jzettle · 2022-09-30T21:45:14Z

This PR adds offbeam gate counting into the CAFs. The number of gates is computed as a difference with the gate counting performed within the event to compute the number of offbeam gates for BNB or NuMI seen by the trigger hardware on an event-by-event basis.

… are handled

jzettle · 2022-09-30T21:54:27Z

This is work largely done a month ago but I missed making the PR then. This is a set of PRs that spans SBNSoftware/sbnana#89, SBNSoftware/sbnobj#67, and SBNSoftware/sbnanaobj#79

jzennamo · 2022-10-03T19:55:32Z

Big picture, it looks like you have added the EXT accounting to sbncode/BeamSpillInfoRetriever/BNBRetriever/BNBRetriever_module.cc is that true? If not could you describe what you're changing in each file? Thanks!

miquelnebot · 2022-10-06T11:11:00Z

trigger build SBNSoftware/sbnobj#67 SBNSoftware/sbnanaobj#79

FNALbuild · 2022-10-06T11:14:17Z

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

FNALbuild · 2022-10-06T11:14:19Z

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

jzettle · 2022-10-06T11:24:43Z

@jzennamo Sorry for the delay. Yeah the latest commit from me cleans that up that is the way I had it initially and then realized is was better to use a separate module. That should not be the case and is not in my working version. Most of the details here are in CAFMaker that adds a connection to the EXTRetriever module which fills an object like the BNB/NuMIRetriever which the number of offbeam gates. The other PR in the set of 4 add the infrastructure needed to support this. I've tested this on the data and it appears to work.

FNALbuild · 2022-10-06T11:24:55Z

❌ 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 · 2022-10-06T11:35:22Z

❌ 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

FNALbuild · 2022-10-06T11:35:58Z

❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for e20: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 · 2022-10-06T11:36:27Z

❌ CI build for SBND Failed at phase build SBND on slf7 for e20: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

miquelnebot · 2022-10-12T13:38:44Z

@jzettle could you take a look to the CI errors, please?

jzettle · 2022-10-12T14:11:27Z

@miquelnebot Will do ASAP. Took a quick look now and I need to understand better what the error means

…expected one for all events

….com/SBNSoftware/sbncode into feature/jzettle_OffBeamGateCountCAFs

jzettle · 2022-10-12T21:10:30Z

@miquelnebot I see you merged the branch in this repo (sbncode) with develop before running the trigger build command. I do not see that done in the other repos. This branch at the time you performed that action is based on an older version than develop (v09_59_00) Is that causing a mismatch somewhere when building?

miquelnebot · 2022-10-13T12:16:57Z

@jzettle GitHub flags you "This branch is out-of-date with the base branch" and asks you to either update or rebase (in this case rebase can not be done due to conflicts)
Is this changing any of your developments? if so, could you resolve the conflicts?

jzettle · 2022-10-13T13:15:29Z

I honestly have no idea, I thought I resolved this with all the commits I am able to build and run this in my own area with no problems and git did not tell me I had anymore conflicts after I did this last round of commits. So I don't understand where the issue is with git here.

jzennamo · 2022-10-13T21:06:30Z

Looks good!

miquelnebot · 2022-10-17T10:22:38Z

trigger build SBNSoftware/sbnobj#67 SBNSoftware/sbnanaobj#79

FNALbuild · 2022-10-17T10:26:03Z

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

FNALbuild · 2022-10-17T10:35:59Z

❌ 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 · 2022-10-17T10:37:07Z

❌ 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

FNALbuild · 2022-10-17T11:02:34Z

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

FNALbuild · 2022-10-17T11:21:26Z

❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for e20: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 · 2022-10-17T11:22:10Z

❌ CI build for SBND Failed at phase build SBND on slf7 for e20: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

PetrilloAtWork · 2022-10-19T20:49:05Z

The error is originating from the changes in sbnanaobj. I have left a review comment in there.

PetrilloAtWork

Some comments documenting the meaning of the variables would help.
No shining issue, but for once I yield to @jzennamo (and maybe also Gray?) for design comments.

PetrilloAtWork · 2022-10-08T09:19:36Z

+    if(triggerInfo.gate_type == 1)
+      triggerInfo.number_of_gates_since_previous_event = frag.getDeltaGatesBNB();
+    else 
+    {
+      triggerInfo.number_of_gates_since_previous_event = frag.getDeltaGatesOther();
+      mf::LogDebug("BNBRetriever") << "Possibly unsupported gate type: " << triggerInfo.gate_type << "! Using information stored in other gate type." << std::endl; 
+    }


Since the logic is identical to the one of the next if branch, I would merge them, something like:

Suggested change

if(triggerInfo.gate_type == 1)

triggerInfo.number_of_gates_since_previous_event = frag.getDeltaGatesBNB();

else

{

triggerInfo.number_of_gates_since_previous_event = frag.getDeltaGatesOther();

mf::LogDebug("BNBRetriever") << "Possibly unsupported gate type: " << triggerInfo.gate_type << "! Using information stored in other gate type." << std::endl;

}

if(triggerInfo.gate_type == 1) {

triggerInfo.number_of_gates_since_previous_event = frag.getDeltaGatesBNB();

triggerInfo.t_previous_event = (static_cast<double>(frag.getLastTimestampBNB()))/(1e9);

}

else

{

triggerInfo.number_of_gates_since_previous_event = frag.getDeltaGatesOther();

triggerInfo.t_previous_event = (static_cast<double>(frag.getLastTimestampOther()))/(1000000000.0);

mf::LogDebug("BNBRetriever") << "Possibly unsupported gate type: " << triggerInfo.gate_type << "! Using information stored in other gate type." << std::endl;

}

PetrilloAtWork · 2022-10-08T09:22:20Z

+    if(gate_type == 2)
+      number_of_gates_since_previous_event = frag.getDeltaGatesNuMI();
+    else
+    {
+      number_of_gates_since_previous_event = frag.getDeltaGatesOther();
+      //mf::LogDebug("NuMIRetriever") << "Possibly unsupported gate type: " << gate_type << "! Using information stored in gate type labeled other." << std::endl;
+    }


Same comment as above.

PetrilloAtWork · 2022-10-08T09:24:18Z


- simulate: [extinfo ]
+ simulate: [ extinfo ]
+ trigger_paths: [ simulate  ]


art does not require the definition of trigger_paths when there is only one producer path defined.
It does not hurt to leave it though.

PetrilloAtWork · 2022-10-08T09:27:24Z

  double fPrescaleEvents;
  std::vector<caf::SRBNBInfo> fBNBInfo; ///< Store detailed BNB info to save into the first StandardRecord of the output file
  std::vector<caf::SRNuMIInfo> fNuMIInfo; ///< Store detailed NuMI info to save into the first StandardRecord of the output file
+  std::vector<caf::SREXTInfo> fEXTInfo;


The description of the meaning of this field is missing.

PetrilloAtWork · 2022-10-19T20:50:46Z

  rec.hdr.fno     = fFileNumber;
  if(fFirstInFile)
  {
+    rec.hdr.pot   = fSubRunPOT;


Already present at the end of this scope.

Suggested change

rec.hdr.pot = fSubRunPOT;

miquelnebot · 2022-10-24T11:59:56Z

@jzettle can you take a look at the remaining comments in all related PRs, please?

jzettle · 2022-10-24T18:14:43Z

@miquelnebot Sorry a lot going on, will try to do today. Watch this space

miquelnebot · 2022-11-09T14:19:14Z

@jzettle any chance you can update this? Or maybe a timeline estimate?

jzettle · 2022-11-09T16:21:36Z

Hi Miquel, I can update this I am sorry I had a lot of other things to do in the past week or tow. There was a thought to overhaul this to use the decoded data products instead of the raw data. I want to understand if this is an option that is realizable on a short timescale or whether it is best to implement this later. Thank you, Jacob

…

________________________________ From: Miquel Nebot ***@***.***> Sent: Wednesday, November 9, 2022 8:19 AM To: SBNSoftware/sbncode ***@***.***> Cc: Jacob C. Zettlemoyer ***@***.***>; Mention ***@***.***> Subject: Re: [SBNSoftware/sbncode] Adding offbeam gate counting into the CAFs (PR #298) @jzettle<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_jzettle&d=DwMCaQ&c=gRgGjJ3BkIsb5y6s49QqsA&r=KTT_6JSFl-XwauoN-YpOWeMrhyVzjD6waXUNJPZvYCY&m=L3A6oPA-0Vv8trYnw1ukd-ENpYbXfTV2MQBjJuhR5Kurg18mQEETid_3TEKkwm5V&s=waxwtfyWuWxCJfaaXJGk_4eecCPX498B6xFSIp0skr4&e=> any chance you can update this? Or maybe a timeline estimate? — Reply to this email directly, view it on GitHub<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_SBNSoftware_sbncode_pull_298-23issuecomment-2D1308835666&d=DwMCaQ&c=gRgGjJ3BkIsb5y6s49QqsA&r=KTT_6JSFl-XwauoN-YpOWeMrhyVzjD6waXUNJPZvYCY&m=L3A6oPA-0Vv8trYnw1ukd-ENpYbXfTV2MQBjJuhR5Kurg18mQEETid_3TEKkwm5V&s=-kdLL4vMnX4pCLK-wIEOdiS_BwXy3-ZsECDs6a0dJNE&e=>, or unsubscribe<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AQAAQZ5GC3K4EW45EUXQWKDWHOXG3ANCNFSM6AAAAAAQ2EIUME&d=DwMCaQ&c=gRgGjJ3BkIsb5y6s49QqsA&r=KTT_6JSFl-XwauoN-YpOWeMrhyVzjD6waXUNJPZvYCY&m=L3A6oPA-0Vv8trYnw1ukd-ENpYbXfTV2MQBjJuhR5Kurg18mQEETid_3TEKkwm5V&s=AwwcrqsXc5fTo7nX8jMrv6TPCYW11XupNHj1xbn6mfE&e=>. You are receiving this because you were mentioned.Message ID: ***@***.***>

miquelnebot · 2023-02-03T17:39:55Z

@jzettle can you update this, give an estimated timeline or convert it to a draft, please?
also with the related PRs, please

= added 3 commits August 25, 2022 16:38

Implementation of offbeam gate counter in CAF similar to how BNB/NuMI…

d94a7be

… are handled

Merge with v9_59_00

6aae371

Cleanup BNBRetriever module

59b62f5

jzettle requested a review from jzennamo September 30, 2022 21:45

This was referenced Sep 30, 2022

Adding offbeam gate counting in the CAFs SBNSoftware/sbnobj#67

Closed

Adding offbeam gate counting in CAFs SBNSoftware/sbnanaobj#79

Closed

Adding offbeam gate into the CAFs SBNSoftware/sbnana#89

Draft

jzennamo requested a review from PetrilloAtWork October 3, 2022 19:53

Merge branch 'develop' into feature/jzettle_OffBeamGateCountCAFs

90ebaa8

= added 3 commits October 12, 2022 15:14

Addressing review comments

6399bda

Modules now throw an exception if trigger gate type value is not the …

2c3df8c

…expected one for all events

Merge branch 'feature/jzettle_OffBeamGateCountCAFs' of https://github…

fb2dfa4

….com/SBNSoftware/sbncode into feature/jzettle_OffBeamGateCountCAFs

jzennamo approved these changes Oct 13, 2022

View reviewed changes

PetrilloAtWork reviewed Oct 19, 2022

View reviewed changes

jzettle added the help wanted Extra attention is needed label Feb 3, 2023

jzettle marked this pull request as draft February 3, 2023 17:47

jzettle closed this Jul 17, 2023

Conversation

jzettle commented Sep 30, 2022

Uh oh!

jzettle commented Sep 30, 2022

Uh oh!

jzennamo commented Oct 3, 2022

Uh oh!

miquelnebot commented Oct 6, 2022

Uh oh!

FNALbuild commented Oct 6, 2022

Uh oh!

FNALbuild commented Oct 6, 2022

Uh oh!

jzettle commented Oct 6, 2022

Uh oh!

FNALbuild commented Oct 6, 2022

Uh oh!

FNALbuild commented Oct 6, 2022

Uh oh!

FNALbuild commented Oct 6, 2022

Uh oh!

FNALbuild commented Oct 6, 2022

Uh oh!

miquelnebot commented Oct 12, 2022

Uh oh!

jzettle commented Oct 12, 2022

Uh oh!

jzettle commented Oct 12, 2022

Uh oh!

miquelnebot commented Oct 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jzettle commented Oct 13, 2022

Uh oh!

jzennamo commented Oct 13, 2022

Uh oh!

miquelnebot commented Oct 17, 2022

Uh oh!

FNALbuild commented Oct 17, 2022

Uh oh!

FNALbuild commented Oct 17, 2022

Uh oh!

FNALbuild commented Oct 17, 2022

Uh oh!

FNALbuild commented Oct 17, 2022

Uh oh!

FNALbuild commented Oct 17, 2022

Uh oh!

FNALbuild commented Oct 17, 2022

Uh oh!

PetrilloAtWork commented Oct 19, 2022

Uh oh!

PetrilloAtWork left a comment

Choose a reason for hiding this comment

Uh oh!

PetrilloAtWork Oct 8, 2022

Choose a reason for hiding this comment

Uh oh!

PetrilloAtWork Oct 8, 2022

Choose a reason for hiding this comment

Uh oh!

PetrilloAtWork Oct 8, 2022

Choose a reason for hiding this comment

Uh oh!

PetrilloAtWork Oct 8, 2022

Choose a reason for hiding this comment

Uh oh!

PetrilloAtWork Oct 19, 2022

Choose a reason for hiding this comment

Uh oh!

miquelnebot commented Oct 24, 2022

Uh oh!

jzettle commented Oct 24, 2022

Uh oh!

miquelnebot commented Nov 9, 2022

Uh oh!

jzettle commented Nov 9, 2022 via email

Uh oh!

miquelnebot commented Feb 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

miquelnebot commented Oct 13, 2022 •

edited

Loading

miquelnebot commented Feb 3, 2023 •

edited

Loading