Feature/blip energy drift correction by Jjm321814 · Pull Request #941 · SBNSoftware/sbndcode

Jjm321814 · 2026-07-10T15:09:31Z

Adding additional blip energy variables so analyzers have access to a drift corrected energy, as well as an explicitly not drift corrected one.

All drift corrections are anchored to the event timestamp, so the beam blips will come out with the right energy. Any out-of-time blip activity will be mis-corrected, so the noDriftCorrection variables are saved as new variables.

Merge branch 'BugFix/BlipADCScale' into feature/BlipEnergyDriftCorrection

Jjm321814 · 2026-07-10T15:14:46Z

This PR is part of a set of 4 in sbndcode, sbnobj, sbncode, and sbnanaobj to make this new blip data entry, and share it to CAF level.
Each PR is at
sbndcode: #941
sbnobj: SBNSoftware/sbnobj#174
sbncode: SBNSoftware/sbncode#661
sbnanaobj: SBNSoftware/sbnanaobj#193

Jjm321814 · 2026-07-10T15:20:12Z

Initial checks on MC looked good. The values of energy were very mildly shifted with temporary debug print outs like

 dep El before corr 1767.8
0.0615994
 dep El after corr 1767.88
Blip Time1.55457 Det Lifetime 35000
0.0615994
 finally blip energy 0.0616021

Unfortunately the detector properties service used to get the electron lifetime initially applies a fixed lifetime. We will need to connect to some kind of calibration database service to get better results on data.

…sily shared

…ol easier to import

Jjm321814 · 2026-07-13T20:14:26Z

This PR is more complicated.
Applying the blip drift correction in data required some interface to the NormalizeDriftSQLite_tool. That was added in to the blipAlg files.
But I don't know how to use an art_tool, so I changed the implementation of NormalizeDriftSQLite_tool so it has an additional class implementation that can be imported more easily.
To avoid code duplication I split the actual definition and implementation code into their own files that get imported. CMakeLists needed updated to accommodate these changes.

This PR does not interrupt PANDORA's use of the NormalizeDriftSQLite_tool and allows blips to use NormalizeDriftSQLite_class. If we have an example of implementation of the art_tool version (that doesn't result in the code duplication seen in

sbndcode/sbndcode/Calorimetry/LightCaloProducer_module.cc

Lines 821 to 848 in 27d88f5

    
           sbnd::LightCaloProducer::ELifetimeInfo sbnd::LightCaloProducer::GetELifetimeFromDB(uint64_t run) { 
        
             // Check cache first 
        
             if (felifetime_cache.count(run)) { 
        
               return felifetime_cache.at(run); 
        
             } 
        
             // Query database - translate run into fake "timestamp"  
        
             // (same convention as NormalizeDriftSQLite) 
        
             felifetime_db->UpdateData((run + 1000000000) * 1000000000); 
        
             ELifetimeInfo info; 
        
             double tau_E, tau_W; 
        
             felifetime_db->GetNamedChannelData(0, "etau_sce_spatial_east", tau_E); 
        
             felifetime_db->GetNamedChannelData(0, "etau_sce_spatial_west", tau_W); 
        
             info.tau_tpc0 = tau_E*1e3; // the db value is in ms, convert to us 
        
             info.tau_tpc1 = tau_W*1e3; // the db value is in ms, convert to us 
        
             if (fverbose) { 
        
               std::cout << "[LightCaloProducer] : Electron lifetime from DB for run " << run << std::endl; 
        
               std::cout << "[LightCaloProducer] : TPC0 (East): " << info.tau_tpc0 << " us" << std::endl; 
        
               std::cout << "[LightCaloProducer] : TPC1 (West): " << info.tau_tpc1 << " us" << std::endl; 
        
             } 
        
             // Cache the result 
        
             felifetime_cache[run] = info; 
        
             return info; 
        
           }

) I'd be happy to simplify this PR to that suggestion.

Jjm321814 · 2026-07-13T20:15:52Z

Oh oops let me get rid of the print statements I used to check the implementation.

Jjm321814 · 2026-07-13T20:18:59Z

I have verified this code imports the correct lifetime values and adds additional variables to the blip larsoft output (reco2) and SRBlip output (CAF).
The validation was done on 10 data events from one run and 10 MC events.

Jjm321814 · 2026-07-30T16:11:59Z

I need to include normalizeDrift settings in the blipconfig.fcl (never pushed those changes)

…n' into feature/BlipEnergyDriftCorrection

PetrilloAtWork

Left a few comments. I am particularly confused by the pattern of the tool source code.
However, I bear no authority on sbndcode, so mine should be taken just as suggestions.

PetrilloAtWork · 2026-07-14T18:16:29Z

    fCylinderRadius     = pset.get<float>         ("CylinderRadius",      15);

    fCaloAlg            = new calo::CalorimetryAlg( pset.get<fhicl::ParameterSet>("CaloAlg") );
+    ElifetimeTool       = new sbnd::calo::NormalizeDriftSQLite( pset.get<fhicl::ParameterSet>("NormalizeDrift"));


I recommend the use of std::unique_ptr here ([CF-051]). In shared SBN code, we forbid the use of new/delete, but this is not SBN code so it's up to you (I would change also the calo algorithm, rather than abiding to its questionable example).
Actually, I would go directly with an object in the class: I currently don't see a reason to allocate it dynamically; but that has also to do with a plan of future extension to other implementations.

PetrilloAtWork · 2026-07-14T18:19:44Z

    fESTAR_p0           = pset.get<float>         ("ESTAR_p0",            0.01730);
    fESTAR_p1           = pset.get<float>         ("ESTAR_p1",            0.00003479);
-    fLifetimeCorr       = pset.get<bool>          ("LifetimeCorrection",  false);
+    fLifetimeCorr       = pset.get<bool>          ("LifetimeCorrection",  true);


Make sure the change of default is well advertised (also in release notes, if SBND maintains any).

PetrilloAtWork · 2026-07-14T18:22:35Z

      float Efield  = kNominalEfield;
-
+      float recomb  = ModBoxRecomb(fCalodEdx,Efield);
+      blip.EnergyNoDriftCorrection   = depEl * (1./recomb) * kWion;


Why not just

Suggested change

blip.EnergyNoDriftCorrection = depEl * (1./recomb) * kWion;

blip.EnergyNoDriftCorrection = depEl / recomb * kWion;

?
No big deal, but it feels strange.

PetrilloAtWork · 2026-07-14T18:24:01Z

+      float energy_estar = Q_to_E_ESTAR(depEl);
+      float energy_pstar = Q_to_E_PSTAR(depEl);
+      blip.EnergyESTARNoDriftCorrection  = energy_estar;
+      blip.EnergyPSTARNoDriftCorrection  = energy_pstar;


It does not look like you really need that temporary parking spot any more:

Suggested change

float energy_estar = Q_to_E_ESTAR(depEl);

float energy_pstar = Q_to_E_PSTAR(depEl);

blip.EnergyESTARNoDriftCorrection = energy_estar;

blip.EnergyPSTARNoDriftCorrection = energy_pstar;

blip.EnergyESTARNoDriftCorrection = Q_to_E_ESTAR(depEl);

blip.EnergyPSTARNoDriftCorrection = Q_to_E_PSTAR(depEl)r;

PetrilloAtWork · 2026-07-14T20:00:55Z

+      energy_estar = Q_to_E_ESTAR(depEl);
+      energy_pstar = Q_to_E_PSTAR(depEl); //reaculate after drift correction
      blip.EnergyESTAR = energy_estar;
      blip.EnergyPSTAR = energy_pstar;


Suggested change

energy_estar = Q_to_E_ESTAR(depEl);

energy_pstar = Q_to_E_PSTAR(depEl); //reaculate after drift correction

blip.EnergyESTAR = energy_estar;

blip.EnergyPSTAR = energy_pstar;

blip.EnergyESTAR = Q_to_E_ESTAR(depEl);

blip.EnergyPSTAR = Q_to_E_PSTAR(depEl); //recalculate after drift correction

PetrilloAtWork · 2026-08-11T23:44:26Z

+//#include "art/Framework/Core/EDProducer.h"
+#include "art/Framework/Principal/Event.h"
+#include "art/Framework/Principal/Handle.h"
+#include "art/Framework/Services/Registry/ServiceHandle.h"
+#include "art/Persistency/Common/PtrMaker.h"
+#include "art/Utilities/ToolMacros.h"
+#include "cetlib_except/exception.h"
+#include "cetlib/cpu_timer.h"
+#include "fhiclcpp/ParameterSet.h"
+#include "messagefacility/MessageLogger/MessageLogger.h"
+
+#include "larevt/CalibrationDBI/Providers/DBFolder.h"
+
+// Tool include
+#include "larreco/Calorimetry/INormalizeCharge.h"
+
+// Services
+#include "lardata/DetectorInfoServices/DetectorClocksService.h"
+
+// Lab helpers
+//#include "wda.h"
+
+// C++
+#include <string>
+#include <optional>
+#include <cassert>


These headers should be sorted out... many of these are not used in the header, so they should live in the .cc file.

Suggested change

//#include "art/Framework/Core/EDProducer.h"

#include "art/Framework/Principal/Event.h"

#include "art/Framework/Principal/Handle.h"

#include "art/Framework/Services/Registry/ServiceHandle.h"

#include "art/Persistency/Common/PtrMaker.h"

#include "art/Utilities/ToolMacros.h"

#include "cetlib_except/exception.h"

#include "cetlib/cpu_timer.h"

#include "fhiclcpp/ParameterSet.h"

#include "messagefacility/MessageLogger/MessageLogger.h"

#include "larevt/CalibrationDBI/Providers/DBFolder.h"

// Tool include

#include "larreco/Calorimetry/INormalizeCharge.h"

// Services

#include "lardata/DetectorInfoServices/DetectorClocksService.h"

// Lab helpers

//#include "wda.h"

// C++

#include <string>

#include <optional>

#include <cassert>

#include "art/Framework/Principal/Event.h"

#include "fhiclcpp/ParameterSet.h"

#include "larevt/CalibrationDBI/Providers/DBFolder.h"

// Tool include

#include "larreco/Calorimetry/INormalizeCharge.h"

// Services

#include "lardata/DetectorInfoServices/DetectorClocksService.h"

// C++

#include <string>

#include <optional>

#include <cassert>

#include "art/Utilities/ToolMacros.h" should be moved to the _tool.h header (which is where the tool macros are used).
Headers are needed for recob::Hit (lardataobj/RecoBase/Hit.h), geo::Point_t (larcoreobj/SimpleTypesAndConstants/geo_vectors.h), std::map (map) and uint32_t (cstdint?).

PetrilloAtWork · 2026-08-11T23:56:13Z

 cet_build_plugin(NormalizeDriftSQLite art::tool LIBRARIES ${TOOL_LIBRARIES})
 cet_build_plugin(NormalizeYZ art::tool LIBRARIES ${TOOL_LIBRARIES})
-
+cet_make_library(LIBRARY_NAME sbndcode_GIMME_LIFETIMES


While GIMME_LIFETIMES is indeed pretty straightforward and self-describing, it may complicate users' lifetime. At a certain point, from other code or from the documentation, I find out that I need the class sbnd::calo::NormalizeDriftSQLite, and I need to add its library. Normally what I would do is to look for sbndcode::NormalizeDriftSQLite or sbndcode::‎Calibration/TPCCalorimetry or sbndcode::‎Calibration/TPCCalorimetry/NormalizeDriftSQLite, but I will never guess GIMME_LIFETIMES.

PetrilloAtWork · 2026-08-11T23:57:27Z

I am confused by this pattern: what is the reason for a _Definitions.h and a _class.h to coexist? what about _Implementation.cc and _class.cc?

PetrilloAtWork · 2026-08-12T00:01:41Z

 ####################################
 product		version		qual	flags		<table_format=2>
-sbncode		v10_21_00       -
+sbncode		v10_21_02       -


Usually version bumps are not added to the PR, especially for merge in develop.
The reason: imagine that now a picky, annoying reviewer (ah-rum) starts giving you hard time for petty reasons. It takes one month to convince him to move forward. At that point, develop is using sbncode v10_23_00: this PR will create a conflict and will need to be fixed. Better to leave this type of updates to the release managers if needed (usually they are not).

Suggested change

sbncode v10_21_02 -

sbncode v10_21_00 -

Jacob McLaughlin and others added 5 commits July 6, 2026 13:20

typo in path

dffdf86

Added new blip energy records

40b1197

:q

599d3ec

Merge branch 'BugFix/BlipADCScale' into feature/BlipEnergyDriftCorrection

turning on lifetime correction by default

3d01043

thought I fixed this

0ea14ac

This was referenced Jul 10, 2026

Added new blip energy records SBNSoftware/sbnobj#174

Open

Added new blip energy records SBNSoftware/sbnanaobj#193

Open

Feature/blip energy drift correction SBNSoftware/sbncode#661

Open

Jjm321814 and others added 14 commits July 10, 2026 12:43

Updated Blip Lifetime lookups

547f42a

A best guess at CMAKE. This Normalize drift tool is not made to be ea…

e005f72

…sily shared

missed a ;

d4ad1fe

Moving useful tools to public

4ca4634

fixed two typo

b519cf4

Guessed wrong on camkelist. We should make the E-Lifetime database to…

3e6d2ff

…ol easier to import

trying other cmake links

c974ccf

sorting out compiler issues

36c5b2b

missed this file

9bc90b7

maybe this stops two objects from being made

c1d8197

Is it double including EDproducer? Surely not

c0174c6

A scuffed working implementation. I'll try to fix in next

f4ff8b7

trying to remove code duplication

9819d3f

Think this is a slightly nicer implementation. Will check if it runs

fedb0a8

Jjm321814 changed the title ~~Draft: Feature/blip energy drift correction~~ Feature/blip energy drift correction Jul 13, 2026

removed debug

f651fd6

Jjm321814 mentioned this pull request Jul 30, 2026

Feature/blip energy driftcorrection production version #952

Open

Jacob McLaughlin added 2 commits July 30, 2026 12:11

missed a fcl config

2d6f4a0

Merge remote-tracking branch 'origin/feature/BlipEnergyDriftCorrectio…

39f511f

…n' into feature/BlipEnergyDriftCorrection

Jjm321814 requested review from PetrilloAtWork, justinjmueller, miquelnebot and sungbinoh July 30, 2026 17:18

PetrilloAtWork reviewed Aug 12, 2026

View reviewed changes

	blip.EnergyNoDriftCorrection = depEl * (1./recomb) * kWion;
	blip.EnergyNoDriftCorrection = depEl / recomb * kWion;

Uh oh!

Conversation

Jjm321814 commented Jul 10, 2026

Uh oh!

Jjm321814 commented Jul 10, 2026

Uh oh!

Jjm321814 commented Jul 10, 2026

Uh oh!

Jjm321814 commented Jul 13, 2026

Uh oh!

Jjm321814 commented Jul 13, 2026

Uh oh!

Jjm321814 commented Jul 13, 2026

Uh oh!

Jjm321814 commented Jul 30, 2026

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants