Enables Database Lookup for Exposure accounting #394

jzennamo · 2023-11-01T21:10:55Z

No description provided.

jzennamo · 2023-11-01T21:14:04Z

This is connected to:
#394

PetrilloAtWork

For BNB, the changes are aimed to skip the gates which have been exposed to the minimum bias trigger (and as such have been acquired). Whether a spill is acquired as minimum bias is discovered consulting an external database which was filled separately with trigger information.
This information should have been included in the pull request description.
The code has some hard-coded unexplained values, which need to be documented. The code does not specifically treat cases where the spill is not in the database. Most spills in fact won't be in the database, since the database has entries only for gates which triggered — it is however expected that all the minimum bias spills are present, because they did trigger. This subtlety might be worth documenting too.
I am not comfortable to have production code rely on a database currently stored in /pnfs/icarus/scratch, and much less comfortable that such location is hard-coded so that fixing it forces a new code release. I strongly recommend that to be moved into a parameter now.
For the NuMI module, GIT is not very helpful in highlighting the differences... I notice the change from V2 to V3 (and consequent metadata interface change). Is there anything else that I missed?
For the BNB off-beam module, I did not understand the meaning of the code. Please explain.

sbncode/BeamSpillInfoRetriever/BNBRetriever/BNBRetriever_module.cc

PetrilloAtWork · 2023-11-21T19:34:23Z

sbncode/BeamSpillInfoRetriever/job/bnbspillinfo.fcl

    Bundle: "BoosterNeutrinoBeam_read"
    MultiWireBundle: "BNBMultiWire"
-    TimeWindow: "500" #seconds
+    TimeWindow: "700" #seconds


We need comments to document where these values come from.

This was updated because with the trigger database we were able to actually realize we were not grabbing all the spills, that additional 200 seconds has allowed us to make sure we are grabbing all the correct spills between the two majority triggers

PetrilloAtWork · 2023-11-21T19:39:49Z

sbncode/BeamSpillInfoRetriever/BNBRetriever/BNBRetriever_module.cc

+    //check if this spill is is minbias   
+    mf::LogDebug("BNBRetriever") << std::setprecision(19) << "matchMultiWireData:: trigger type : " << get_trigger_type_matching_gate(db, callback_trigger_type, run_number, times_temps[i]*1.e9-triggerInfo.WR_to_Spill_conversion+3.6e7, 40.) << " times : spill " << times_temps[i]*1.e9 << " - " << triggerInfo.WR_to_Spill_conversion << " + " << 3.6e7 <<  std::endl;

+    if(get_trigger_type_matching_gate(db, callback_trigger_type, run_number, times_temps[i]*1.e9-triggerInfo.WR_to_Spill_conversion+3.6e7, 40.) == 1){


This 40 (milliseconds) also needs explanation, i.e. why we choose 40 ms of tolerance.
Also, this is a real value, while the threshold parameter of get_trigger_type_matching_gate() is an integer.

40 ms was selected to be close to but outside the 66 ms time of the next spill (when the beam is running at 15 Hz) DocDB 33155 provides documentation of this, I have resolved the int vs float issue and added a comment

PetrilloAtWork · 2023-11-21T19:49:26Z

sbncode/BeamSpillInfoRetriever/BNBRetriever/BNBRetriever_module.cc

  produces< std::vector< sbn::BNBSpillInfo >, art::InSubRun >();
  TotalBeamSpills = 0;
+
+  rc = sqlite3_open("/pnfs/icarus/scratch/users/mueller/icarus_triggers.db", &db);


Hard coded path to scratch area in the code won't do.
Either move the file to stable area, or set the path to a parameter.
Better: both.

Moved file to sbndata and made the path a fcl parameter

sbncode/BeamSpillInfoRetriever/BNBRetriever/BNBRetriever_module.cc

PetrilloAtWork · 2023-11-23T02:56:02Z

sbncode/BeamSpillInfoRetriever/BNBEXTRetriever/BNBEXTRetriever_module.cc

  // art::SubRun so it persists 
+
+  if(evtCount != 0 && totalMinBias != 0)
+    scale_factor = 1. - (evtCount/totalMinBias);


I can't figure out the rationale here.
Can you explain (and document) the intent?

We need to scale by the minimum bias prescale rate, which can change run-to-run based on our configuration. The best way I found to do this was to use frag.getDeltaGatesBNBOffMinbias() BUT it doesn't work for event which haven't had a MinBias trigger yet.

You can get the prescale rate with something like:

void sbn::BNBExtRetriever::beginRun(art::Run& run) { auto const& trigConfig = run.getProduct<icarus::TriggerConfiguration>(fTrigConfigLabel); fPrescale = trigConfig.getMinBiasPrescale(sbn::triggerSource::OffbeamBNB); }

(#include "icaruscode/Decode/DataProducts/TriggerConfiguration.h")
With this information you can correct either statistically (* (1.0 - 1.0/fPrescale)) or exactly event by event.
The correct label from ICARUS Stage0 processing is triggerConfig if I am not wrong.

I don't understand the meaning of the fraction evtCount/totalMinBias... I would understand a gate count instead of the triggered event count on the denominator as an estimation of the prescale factor.

I am not sure what the existing code does in the presence of events with gate_type != 3.

SFBayLaser · 2023-11-29T17:04:39Z

I am very concerned about the need for an external database which is currently at some 8 GB from anecdotes on a slack thread. We need to find a place to house this, and determine how it is used, in a way that does not break general users/developers and/or prevent them from running standard workflows, potentially in non-gpvm environments.

etworcester · 2023-12-20T15:49:19Z

Documenting in-person discussion from yesterday. @justinjmueller believes he can extract a database with only the relevant information that will be small enough to live in icarusdata so the offline database issue is resolved. (The issue of what to do about online filling of a database that would be available for keepup production in the future is left for another day).

justinjmueller · 2023-12-20T15:55:43Z

The database size has been reduced to ~84 MB after removing unnecessary columns and any triggers which are not on-beam triggers. @jzennamo has said that he will find some time for testing the reduced database to make sure there are no problems.

…he spill quality for the spill that goes with a given event, and code supporting that.

…wants to use this spill. The other checks not just for the first event but the first event OF A GIVEN STREAM for the numi case.

…add another). Thanks Gianluca

…nyway :)

jzennamo · 2024-01-11T18:08:35Z

OK, I have gotten @justinjmueller's new database working and it is rather quick! I am going to start working through @PetrilloAtWork's comments now. Thanks everyone!

PetrilloAtWork

Following up on one of the comments.
Maybe I am using GitHub wrong.

PetrilloAtWork · 2024-01-11T23:53:02Z

sbncode/BeamSpillInfoRetriever/BNBEXTRetriever/BNBEXTRetriever_module.cc

  // art::SubRun so it persists 
+
+  if(evtCount != 0 && totalMinBias != 0)
+    scale_factor = 1. - (evtCount/totalMinBias);


You can get the prescale rate with something like:

void sbn::BNBExtRetriever::beginRun(art::Run& run) { auto const& trigConfig = run.getProduct<icarus::TriggerConfiguration>(fTrigConfigLabel); fPrescale = trigConfig.getMinBiasPrescale(sbn::triggerSource::OffbeamBNB); }

(#include "icaruscode/Decode/DataProducts/TriggerConfiguration.h")
With this information you can correct either statistically (* (1.0 - 1.0/fPrescale)) or exactly event by event.
The correct label from ICARUS Stage0 processing is triggerConfig if I am not wrong.

I don't understand the meaning of the fraction evtCount/totalMinBias... I would understand a gate count instead of the triggered event count on the denominator as an estimation of the prescale factor.

I am not sure what the existing code does in the presence of events with gate_type != 3.

SFBayLaser

My concern regarding the home for the database has been nicely resolved so am happy to approve.

jzennamo · 2024-01-17T15:46:35Z

OK! After a good amount of fighting, I have had to update everything to the latest flavor of develop @PetrilloAtWork does it look like all your requested changes are in here?

This PR needs to be paired with these two other PRs:
SBNSoftware/sbnobj#100
SBNSoftware/sbndata#6

PetrilloAtWork

Looks ok.
Thank you for your patience!

miquelnebot · 2024-01-24T16:07:53Z

trigger build SBNSoftware/sbnobj#100 SBNSoftware/sbndata#6

…FOR_DEVELOP_basedOnJZennamoBranch Feature/howard spillquality for develop based on j zennamo branch

FNALbuild · 2024-01-24T16:57:54Z

❌ 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

FNALbuild · 2024-01-24T17:00:50Z

❌ 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

FNALbuild · 2024-01-24T17:03:32Z

❌ 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 · 2024-01-24T17:06:36Z

❌ 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

miquelnebot · 2024-01-26T12:12:58Z

trigger build SBNSoftware/sbnobj#100 SBNSoftware/sbndata#6 SBNSoftware/sbnanaobj#119

FNALbuild · 2024-01-26T12:16:06Z

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

FNALbuild · 2024-01-26T12:16:07Z

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

FNALbuild · 2024-01-26T12:54:43Z

⚠️ CI build for ICARUS Warning at phase ci_tests ICARUS on slf7 for e26:prof - ignored failure for unit_test - 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 · 2024-01-26T13:01:26Z

❌ CI build for SBND Failed at phase ci_tests SBND on slf7 for c14:prof - ignored warnings for build -- details available through the CI dashboard

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

parent CI build details are available through the CI dashboard

FNALbuild · 2024-01-26T13:02:23Z

⚠️ CI build for SBND Warning at phase ci_tests SBND on slf7 for e26: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 · 2024-01-26T13:18:35Z

❌ CI build for ICARUS Failed at phase ci_tests ICARUS on slf7 for c14:prof - ignored failure for unit_test - 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

Joseph Zennamo and others added 5 commits August 23, 2023 13:32

Updated to TriggerV3

92aea80

Updated to Trigger V3

021d19a

Taking into account MinBias Triggers

93935e7

fix bug, forgot to add minbias scale_factor

29eee27

protect against events before minbias trigger

8493e09

jzennamo requested review from PetrilloAtWork and SFBayLaser November 1, 2023 21:10

Merge branch 'develop' into feature/zennamo_AccountingForMinBiasTriggers

bd6cb97

jzennamo mentioned this pull request Nov 1, 2023

Taking into account MinBias Triggers SBNSoftware/sbnobj#100

Merged

PetrilloAtWork requested changes Nov 23, 2023

View reviewed changes

brucehoward-physics added 5 commits January 10, 2024 17:55

Add individual BNBInfo and NuMIInfo to SRHeader to enable saving of t…

b3cc268

…he spill quality for the spill that goes with a given event, and code supporting that.

Fix a few issues - one adds a check on PadTime before deciding if it …

39fe109

…wants to use this spill. The other checks not just for the first event but the first event OF A GIVEN STREAM for the numi case.

Updated code with some of Gianluca P's suggestions (need to test and …

20ba04f

…add another). Thanks Gianluca

Second part of incorporating Gianluca's comments. Thanks

2b6c7ed

Adding to fix a pet peeve of Gianluca since I was re-testing things a…

15f8421

…nyway :)

PetrilloAtWork reviewed Jan 11, 2024

View reviewed changes

SFBayLaser approved these changes Jan 16, 2024

View reviewed changes

jzennamo mentioned this pull request Jan 16, 2024

This PR Adds the ICARUS Run2 Trigger Database SBNSoftware/sbndata#6

Merged

jzennamo added 3 commits January 16, 2024 13:00

adding guard for non-offbeam triggers

31b87be

Merge branch 'develop' into feature/zennamo_AccountingForMinBiasTriggers

b4b6f35

updates with changes

3c21acc

jzennamo mentioned this pull request Jan 17, 2024

Configurations Enabling ICARUS NuMI Analysis #391

Closed

PetrilloAtWork approved these changes Jan 17, 2024

View reviewed changes

Merge pull request #407 from SBNSoftware/feature/howard_spillquality_…

049442b

…FOR_DEVELOP_basedOnJZennamoBranch Feature/howard spillquality for develop based on j zennamo branch

SBNSoftware deleted a comment from FNALbuild Jan 26, 2024

miquelnebot merged commit 9eb364e into develop Jan 26, 2024

Enables Database Lookup for Exposure accounting #394

Enables Database Lookup for Exposure accounting #394

Uh oh!

Conversation

jzennamo commented Nov 1, 2023

Uh oh!

jzennamo commented Nov 1, 2023

Uh oh!

PetrilloAtWork left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

SFBayLaser commented Nov 29, 2023

Uh oh!

etworcester commented Dec 20, 2023

Uh oh!

justinjmueller commented Dec 20, 2023

Uh oh!

jzennamo commented Jan 11, 2024

Uh oh!

PetrilloAtWork left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SFBayLaser left a comment

Choose a reason for hiding this comment

Uh oh!

jzennamo commented Jan 17, 2024

Uh oh!

PetrilloAtWork left a comment

Choose a reason for hiding this comment

Uh oh!

miquelnebot commented Jan 24, 2024

Uh oh!

FNALbuild commented Jan 24, 2024

Uh oh!

FNALbuild commented Jan 24, 2024

Uh oh!

FNALbuild commented Jan 24, 2024

Uh oh!

FNALbuild commented Jan 24, 2024

Uh oh!

miquelnebot commented Jan 26, 2024

Uh oh!

FNALbuild commented Jan 26, 2024

Uh oh!

FNALbuild commented Jan 26, 2024

Uh oh!

FNALbuild commented Jan 26, 2024

Uh oh!

FNALbuild commented Jan 26, 2024

Uh oh!

FNALbuild commented Jan 26, 2024

Uh oh!

FNALbuild commented Jan 26, 2024

Uh oh!

Reviewers

PetrilloAtWork left a comment •

edited

Loading