Skip to content
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
7f7a897
Adding blip data types to sbnobj to help with CAF addition
Jjm321814 Nov 6, 2025
18d8269
Moving blip utils over
Jjm321814 Nov 6, 2025
9751aa3
Moving blip utils
Jjm321814 Nov 6, 2025
1387b2a
trying to remove depends
Jjm321814 Nov 6, 2025
f5c315a
trying to remove depends
Jjm321814 Nov 6, 2025
2ce06f9
Im not sure how to link the dictionaries here anymore
Jjm321814 Nov 6, 2025
2d37c3f
Simplifying includes
Jjm321814 Nov 6, 2025
55d9f70
trying to make a library
Jjm321814 Nov 6, 2025
343d755
Still trying to make a library
Jjm321814 Nov 6, 2025
99a3c01
sorting compiles
Nov 6, 2025
bcbb2aa
compiled version
Nov 10, 2025
5bfaf2c
small fix for sbndcode
Nov 14, 2025
90a5dc2
Merge branch 'develop' into feature/AddingBlipToCAF
Jjm321814 Nov 14, 2025
a535dc9
Small fix for sbndcode addition
Nov 21, 2025
bbd96f7
Added lots of doxygen comments. Need to test geo::Point_t still
Jjm321814 Nov 29, 2025
8b3cb45
changed to default cmake library
Jjm321814 Nov 29, 2025
47f8630
Updating order in classes.h and adding class version index
Jjm321814 Nov 29, 2025
d4991e5
These have to be in a specific order
Jjm321814 Nov 29, 2025
91eba8c
Getting rid of ;
Jjm321814 Nov 29, 2025
83efc10
don't need this one
Jjm321814 Nov 29, 2025
3d4c322
Adding blank lines to be filled by compiler
Jjm321814 Nov 29, 2025
4e06944
Adding blank lines to be filled by compiler
Jjm321814 Nov 29, 2025
5ae8d14
I don't understand this version tracker
Jjm321814 Nov 29, 2025
1fd419a
I was putting things in the wrong place
Jjm321814 Nov 29, 2025
913d94b
fixed stray typedef
Jjm321814 Nov 29, 2025
ec53ab8
Added checksum
Nov 29, 2025
1595e6c
Missed a few comments
Jjm321814 Nov 29, 2025
6194c03
Changed array initilization
Jjm321814 Nov 29, 2025
41e66f4
Removing some classes.h
Jjm321814 Nov 29, 2025
3736ddd
Updated Checksum
Nov 29, 2025
cc24452
Changing class version
Jjm321814 Nov 29, 2025
dfd4d1d
remove unsaved item
Jjm321814 Nov 29, 2025
4c9288e
removed unused headers
Jjm321814 Nov 29, 2025
d460853
Fixed a comment
Jjm321814 Nov 30, 2025
bdd98cb
Merge branch 'develop' into feature/AddingBlipToCAF
Dec 3, 2025
c16b5be
made pinfo position have a capital like all other blip positions
Jjm321814 Jan 5, 2026
f4872e1
Trying to fix runtime error
Jjm321814 Jan 5, 2026
e718325
Trying to fix runtime error
Jjm321814 Jan 5, 2026
6930443
Trying to fix runtime error
Jjm321814 Jan 5, 2026
5b3feee
Trying to fix runtime error
Jjm321814 Jan 5, 2026
db31ab5
Merge branch 'develop' into feature/AddingBlipToCAF
Jan 5, 2026
55f7c5d
removing the thing I added
Jjm321814 Jan 5, 2026
f7b4b84
fixing run time error
Jjm321814 Jan 5, 2026
9524c9e
The geometry wasn't needed. Geo vectors was
Jan 7, 2026
8cd0ca7
Merge branch 'develop' into feature/AddingBlipToCAF
Jjm321814 Feb 10, 2026
b63c719
addressing doxygen comments
Jjm321814 Feb 10, 2026
0f56d1c
copy paste error
Jjm321814 Feb 10, 2026
f7110fa
fix doxygen comment
Jjm321814 Feb 16, 2026
4ba0fb5
Merge branch 'develop' into feature/AddingBlipToCAF
Jjm321814 Feb 23, 2026
b6d0793
Trying to fix CI issues
Jjm321814 Feb 24, 2026
ee45ae3
Missed one file
Jjm321814 Feb 24, 2026
38e4c1c
Revert "Missed one file"
Jjm321814 Feb 24, 2026
0e27710
Revert "Trying to fix CI issues"
Jjm321814 Feb 24, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions sbnobj/SBND/Blip/BlipDataTypes.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
#include "sbnobj/SBND/Blip/BlipDataTypes.h"
174 changes: 174 additions & 0 deletions sbnobj/SBND/Blip/BlipDataTypes.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
#ifndef BLIPDATATYPE
#define BLIPDATATYPE
#include "lardataobj/RecoBase/Hit.h"
#include "nusimdata/SimulationBase/MCParticle.h"
#include <vector>
#include <map>
#include <set>
typedef std::vector<int> vint_t;
typedef std::vector<bool> vbool_t;
typedef std::vector<float> vfloat_t;
typedef std::set<int> si_t;
typedef std::map<int,float> mif_t;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am puzzled by these typedefs.. to me, these make the code less readable, not more. I suggest using the full data types, rather than si_t etc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding to that: avoid putting definitions in the global namespace of a header.
Those will invade every place where that header is included, directly or indirectly.


namespace blip {
const int kNplanes = 3;
const int kNTPCs = 2;


//###################################################
// Data structures
//###################################################

struct ParticleInfo {
simb::MCParticle particle;
int trackId = -9;
int index = -9;
int isPrimary = -9;
int numTrajPts = -9;
double depEnergy = -9;
int depElectrons = -9;
double numElectrons = -9;
double mass = -9;
double E = -9;
double endE = -9;
double KE = -9;
double endKE = -9;
double P = -9;
double Px = -9;
double Py = -9;
double Pz = -9;
double pathLength = -9;
double time = -9;
double endtime = -9;
TVector3 startPoint;
TVector3 endPoint;
TVector3 position;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please document the meaning of these variables in Doxygen format (for example:

  int   trackId           = -9; ///< ID from GEANT4 track.

There are many things that are not obvious: index in which collection? is mass different than the pole mass typically associated to the true particle? is the numTrajPts different from the one you can find in particle? in fact, where does particle come from, the generator or GEANT4? which are the units of energy? is -9 an invalid momentum value for components like Px?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated all structs to include doxygen style comments.
numTrajPts is the same as in simb::MCParticle
So -9 is physical for Px, but -9 MeV/c of momentum is very high for objects making blips.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other design questions:

  • TVector3 should burn in hell be avoided; can we use geo::Point_t from larcoreobj (larcoreobj/SimpleTypesAndConstants/geo_vectors.h)?
  • why do we have vectors for positions but coordinates for the momentum? should we use geo::Vector_t for the momentum vector? (maybe the answer is "no because a momentum has nothing to do with geometry) Can we use a ROOT GenVector? (that is what geo::Vector_t actually is undercover)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the positions have been updated with geo::Point_t. That said in the BlipAlg, and BlipUtil folders of sbndcode we still have some temporary TVector3 used for processing. Those can be removed in the future but that feels outside the scope of this PR.
I don't have a great reason for the momentum to be 4 variables rather than a vector

};

// True energy depositions
struct TrueBlip {
int ID = -9; // unique blip ID
int Cryostat = -9; // Cryostat ID
int TPC = -9; // TPC ID
float Time = -9; // time of particle interaction
int TimeTick = -9; // time tick
float DriftTime = -9; // drift time [us]
float Energy = 0; // energy dep [MeV]
int DepElectrons = 0; // deposited electrons
int NumElectrons = 0; // electrons reaching wires
int LeadG4ID = -9; // lead G4 track ID
int LeadG4Index = -9; // lead G4 track index
int LeadG4PDG = -9; // lead G4 PDG
float LeadCharge = -9; // lead G4 charge dep
TVector3 Position; // XYZ position
mif_t G4ChargeMap;
mif_t G4PDGMap;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Convert the comments into Doxygen format, like this:

Suggested change
int ID = -9; // unique blip ID
int Cryostat = -9; // Cryostat ID
int TPC = -9; // TPC ID
float Time = -9; // time of particle interaction
int TimeTick = -9; // time tick
float DriftTime = -9; // drift time [us]
float Energy = 0; // energy dep [MeV]
int DepElectrons = 0; // deposited electrons
int NumElectrons = 0; // electrons reaching wires
int LeadG4ID = -9; // lead G4 track ID
int LeadG4Index = -9; // lead G4 track index
int LeadG4PDG = -9; // lead G4 PDG
float LeadCharge = -9; // lead G4 charge dep
TVector3 Position; // XYZ position
mif_t G4ChargeMap;
mif_t G4PDGMap;
int ID = -9; ///< unique blip ID
int Cryostat = -9; ///< Cryostat ID
int TPC = -9; ///< TPC ID
float Time = -9; ///< time of particle interaction
int TimeTick = -9; ///< time tick
float DriftTime = -9; ///< drift time [us]
float Energy = 0; ///< energy dep [MeV]
int DepElectrons = 0; ///< deposited electrons
int NumElectrons = 0; ///< electrons reaching wires
int LeadG4ID = -9; ///< lead G4 track ID
int LeadG4Index = -9; ///< lead G4 track index
int LeadG4PDG = -9; ///< lead G4 PDG
float LeadCharge = -9; ///< lead G4 charge dep
TVector3 Position; ///< XYZ position
mif_t G4ChargeMap;
mif_t G4PDGMap;

The maps are the objects that deserve the most explanation: what does the key represent? etc.
"time tick" I suppose of TPC readout? wouldn't hurt to write that explicitly, even if it's kind of obvious.
What is the time scale of that Time? simulation time? electronics time? trigger time? Depending on scale and precision, float (7 significant digits) might be too tight.
The electrons here are an integral type, while in ParticleInfo they are real numbers (which is not as crazy as it sounds, since sometimes simulation splits a cluster of drifting electrons statistically, so fractions of electrons may arise).

};

struct HitInfo {
int hitid = -9;
int cryo = -9;
int tpc = -9;
int plane = -9;
int wire = -9;
int chan = -9;
float amp = -9;
float rms = -9;
int trkid = -9;
int shwrid = -9;
int clustid = -9;
int blipid = -9;
bool ismatch = false;
float integralADC = -999; // [ADCs] from integral
float sigmaintegral = -999;
float sumADC = -999; // [ADCs] from sum
float charge = -999; // [e-]
float peakTime = -999999;
float driftTime = -999999; // [tick]
float gof = -9;
int g4trkid = -9;
int g4pdg = -999;
int g4charge = -999; // [e-]
float g4frac = -99;
float g4energy = -999; // [MeV]
};

struct HitClust {
int ID = -9;
bool isValid = false;
int CenterChan = -999;
int CenterWire = -999;
bool isTruthMatched = false;
bool isMerged = false;
bool isMatched = false;
int DeadWireSep = 99;
int Cryostat = -9;
int TPC = -9;
int Plane = -9;
int NHits = -9;
int NWires = -9;
float ADCs = -999;
float Amplitude = -999;
float Charge = -999;
float SigmaCharge = -999;
float TimeTick = -999;
float Time = -999;
float StartHitTime = -999;
float EndHitTime = -999;
float StartTime = -999;
float EndTime = -999;
float Timespan = -999;
float RMS = -999;
int StartWire = -999;
int EndWire = -999;
int NPulseTrainHits = -9;
float GoodnessOfFit = -999;
int BlipID = -9;
int EdepID = -9;
si_t HitIDs;
si_t Wires;
si_t Chans;
si_t G4IDs;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also here, document what is each of these. Are they just lists with no particular order? or are any of these paired so that the same index describes the same entity?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These list the hists, wires, channels, and G4 particles that contributed to this reconstructed blip. Every contribution gets listed within the blip, but the blips only contain the indecies that contributed to the reconstructed blip.
The order within the set is arbitrary.


std::map<int,TVector3> IntersectLocations;
};

struct Blip {

int ID = -9; // Blip ID / index
bool isValid = false; // Blip passes basic checks
int Cryostat = -9; // Cryostat
int TPC = -9; // TPC
int NPlanes = -9; // Num. matched planes
int MaxWireSpan = -9; // Maximum span of wires on any plane cluster
float TimeTick = -999; // Readout time [ticks]
float Time = -999; // Drift time [us]
float Charge = -9; // Charge on calorimetry plane
float Energy = -999; // Energy (const dE/dx, fcl-configurable)
float EnergyESTAR = -999; // Energy (ESTAR method from ArgoNeuT)
float EnergyPSTAR = -999; // Energy (PSTAR method similar with ESTAR method from ArgoNeuT)
float ProxTrkDist = -9; // Distance to cloest track
int ProxTrkID = -9; // ID of closest track
bool inCylinder = false; // Is it in a cone/cylinder region?

TVector3 Position; // 3D position TVector3
float SigmaYZ = -9.; // Uncertainty in YZ intersect [cm]
float dX = -9; // Equivalent length along drift direction [cm]
float dYZ = -9; // Approximate length scale in YZ space [cm]

// Plane/cluster-specific information
blip::HitClust clusters[kNplanes];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider:

Suggested change
blip::HitClust clusters[kNplanes];
std::array<blip::HitClust, kNplanes> clusters;

That object provides a size() member that is useful in user code.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated


// Truth-matched energy deposition
blip::TrueBlip truth;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turn the documentation in Doxygen format:

Suggested change
int ID = -9; // Blip ID / index
bool isValid = false; // Blip passes basic checks
int Cryostat = -9; // Cryostat
int TPC = -9; // TPC
int NPlanes = -9; // Num. matched planes
int MaxWireSpan = -9; // Maximum span of wires on any plane cluster
float TimeTick = -999; // Readout time [ticks]
float Time = -999; // Drift time [us]
float Charge = -9; // Charge on calorimetry plane
float Energy = -999; // Energy (const dE/dx, fcl-configurable)
float EnergyESTAR = -999; // Energy (ESTAR method from ArgoNeuT)
float EnergyPSTAR = -999; // Energy (PSTAR method similar with ESTAR method from ArgoNeuT)
float ProxTrkDist = -9; // Distance to cloest track
int ProxTrkID = -9; // ID of closest track
bool inCylinder = false; // Is it in a cone/cylinder region?
TVector3 Position; // 3D position TVector3
float SigmaYZ = -9.; // Uncertainty in YZ intersect [cm]
float dX = -9; // Equivalent length along drift direction [cm]
float dYZ = -9; // Approximate length scale in YZ space [cm]
// Plane/cluster-specific information
blip::HitClust clusters[kNplanes];
// Truth-matched energy deposition
blip::TrueBlip truth;
int ID = -9; ///< Blip ID / index
bool isValid = false; ///< Blip passes basic checks
int Cryostat = -9; ///< Cryostat
int TPC = -9; ///< TPC
int NPlanes = -9; ///< Num. matched planes
int MaxWireSpan = -9; ///< Maximum span of wires on any plane cluster
float TimeTick = -999; ///< Readout time [ticks]
float Time = -999; ///< Drift time [us]
float Charge = -9; ///< Charge on calorimetry plane
float Energy = -999; ///< Energy (const dE/dx, fcl-configurable)
float EnergyESTAR = -999; ///< Energy (ESTAR method from ArgoNeuT)
float EnergyPSTAR = -999; ///< Energy (PSTAR method similar with ESTAR method from ArgoNeuT)
float ProxTrkDist = -9; ///< Distance to cloest track
int ProxTrkID = -9; ///< ID of closest track
bool inCylinder = false; ///< Is it in a cone/cylinder region?
TVector3 Position; ///< 3D position TVector3
float SigmaYZ = -9.; ///< Uncertainty in YZ intersect [cm]
float dX = -9; ///< Equivalent length along drift direction [cm]
float dYZ = -9; ///< Approximate length scale in YZ space [cm]
/// Plane/cluster-specific information
blip::HitClust clusters[kNplanes];
/// Truth-matched energy deposition
blip::TrueBlip truth;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated on all structs.


// Prototype getter functions
double X() { return Position.X(); }
double Y() { return Position.Y(); }
double Z() { return Position.Z(); }

};

}
#endif
11 changes: 11 additions & 0 deletions sbnobj/SBND/Blip/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
cet_make_library( LIBRARY_NAME sbndobj_BlipDataTypes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cet_make_library( LIBRARY_NAME sbndobj_BlipDataTypes
cet_make_library(

Library names are very hard to guess. A verbose but effective strategy is to leave it the default, which makes it easily predictable. And the most used convention guarantees at least the first element to be the repository the library belongs to ("sbndobj"?). I request it does not start with sbndobj, which is misleading, and strongly recommend to leave this library name the default:

  • in CMakeLists.txt link lists, people will have to write the annoyingly long sbnobj::SBND_Blip (could actually be worse);
  • when you #include "sbnobj/SBND/Blip/BlipDataTypes.h", you know that the library you need to add follows the path of the header (other example:
    #include "lardataobj/RecoBase/Hit.h"
    #include "nusimdata/SimulationBase/MCParticle.h"
    links to lardataobj::RecoBase and nusimdata::SimulationBase. Presto.)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the library name here and in relevant include locations.

SOURCE
BlipDataTypes.cc
LIBRARIES
lardataobj::RecoBase
nusimdata::SimulationBase
)
art_dictionary(DICTIONARY_LIBRARIES sbndobj_BlipDataTypes)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(update this with the fixed library name)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated here and in other repo

install_headers()
install_source()
install_fhicl()
36 changes: 36 additions & 0 deletions sbnobj/SBND/Blip/classes.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
//
// Build a dictionary.
//
// $Id: classes.h,v 1.8 2010/04/12 18:12:28 Exp $
// $Author: $
// $Date: 2010/04/12 18:12:28 $
//
// Original author Rob Kutschke, modified by wes
//

#include "canvas/Persistency/Common/Wrapper.h"

// data-products
// lardataobj
//#include "lardata/Utilities/AssociationUtil.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is spurious here:

Suggested change
//#include "lardata/Utilities/AssociationUtil.h"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed this

#include "canvas/Persistency/Common/Assns.h"
#include "lardataobj/RecoBase/PFParticle.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused:

Suggested change
#include "lardataobj/RecoBase/PFParticle.h"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

#include "lardataobj/RecoBase/Hit.h"
#include "nusimdata/SimulationBase/MCTruth.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also not needed:

Suggested change
#include "nusimdata/SimulationBase/MCTruth.h"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

#include "sbnobj/SBND/Blip/BlipDataTypes.h"
#include "lardataobj/RecoBase/SpacePoint.h"

//
// Only include objects that we would like to be able to put into the event.
// Do not include the objects they contain internally.
//

template class art::Assns<recob::Hit,blip::Blip,void>;
template class art::Wrapper<art::Assns<recob::Hit,blip::Blip,void> >;
template class std::vector<blip::Blip>;
template class art::Wrapper<std::vector<blip::Blip> >;
template class std::map<int, TVector3>;
template class art::Assns<blip::Blip,recob::Hit,void>;
template class art::Wrapper<art::Assns<blip::Blip,recob::Hit,void> >;
template class art::Assns<blip::Blip,recob::SpacePoint,void>;
template class art::Wrapper<art::Assns<blip::Blip,recob::SpacePoint,void> >;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to bother specifying the void association metadata explicitly — it's a bit distracting, so I suggest the removal. Also, when you instantiate an art::Wrapper<T>, you also implicitly trigger the instantiation of T, so the others are redundant. This works because this is C++; classes_def.xml still needs all of them, unfortunately. Finally, the "partner association" instantiation is also not needed.

Suggested change
template class art::Wrapper<art::Assns<blip::Blip,recob::SpacePoint,void> >;
template class art::Wrapper<std::vector<blip::Blip> >;
template class art::Wrapper<art::Assns<blip::Blip,recob::Hit> >;
template class art::Wrapper<art::Assns<blip::Blip,recob::SpacePoint> >;

I think the map instantiation is also not needed — I am not sure where it comes from, so I might be wrong.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed voids and the map. Seems to build still

14 changes: 14 additions & 0 deletions sbnobj/SBND/Blip/classes_def.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<lcgdict>
<class name="art::Wrapper<std::vector<blip::Blip> >"/>
<class name="blip::Blip"/>
<class name="std::vector<blip::Blip>"/>
<class name="blip::HitClust"/>
<class name="blip::TrueBlip"/>
<class name="std::map<int,TVector3>"/>
<class name="art::Assns<blip::Blip,recob::Hit,void>"/>
<class name="art::Assns<recob::Hit, blip::Blip,void>"/>
<class name="art::Assns<blip::Blip,recob::SpacePoint,void>"/>
<class name="art::Wrapper<art::Assns<blip::Blip,recob::Hit,void> >"/>
<class name="art::Wrapper<art::Assns<recob::Hit,blip::Blip,void> >"/>
<class name="art::Wrapper<art::Assns<blip::Blip,recob::SpacePoint,void> >"/>
</lcgdict>
1 change: 1 addition & 0 deletions sbnobj/SBND/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ add_subdirectory(Commissioning)
add_subdirectory(Trigger)
add_subdirectory(ToF)
add_subdirectory(Timing)
add_subdirectory(Blip)