Skip to content
Open
1 change: 1 addition & 0 deletions sbnobj/Common/Reco/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ cet_make_library(
CNNScore.cc
CRUMBSResult.cc
FlashTriggerPrimitive.cc
LightCalo.cc
MVAPID.cc
MergedTrackInfo.cc
OpT0FinderResult.cc
Expand Down
1 change: 1 addition & 0 deletions sbnobj/Common/Reco/LightCalo.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
#include "sbnobj/Common/Reco/LightCalo.h"
47 changes: 47 additions & 0 deletions sbnobj/Common/Reco/LightCalo.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* @file sbnobj/Common/Reco/LightCalo.h
* @brief Defines data structures for light calorimetry products.
* @author Lynn Tung
* @date December 2nd, 2025
*
*/

#ifndef sbnobj_LightCalo_H
#define sbnobj_LightCalo_H

#include <limits>

namespace sbn{

/**
* @brief A simple class to store reconstructed charge, light, and visible energy.
*/

class LightCalo {
public:
Comment on lines +20 to +21
Copy link
Member

Choose a reason for hiding this comment

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

Since the whole class content is public, I would declare it struct:

Suggested change
class LightCalo {
public:
struct LightCalo {

It is 100% equivalent, and I think clearer.


// NaN value to initialize data members
static constexpr double nan = std::numeric_limits<double>::signaling_NaN();

/// @name Reconstructed charge, light, and visible energy data members.
/// @{
double charge = nan; ///< Total charge in a reconstructed interaction (recob::Slice) [# of electrons]
double light = nan; ///< Total light in a reconstructed interaction (recob::Slice + recob::OpFlash) [# of photons]
double energy = nan; ///< Total visible energy (sum of charge+light) for a reconstructed interaction [GeV]
int bestplane = -1; ///< Plane that was used for the total charge
/// @}

/**
* Default constructor.
*/
LightCalo() = default;
LightCalo(double charge, double light, double energy, int bestplane)
: charge(charge)
, light(light)
, energy(energy)
, bestplane(bestplane)
{}
Comment on lines +34 to +43
Copy link
Member

Choose a reason for hiding this comment

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

You have introduced a value constructor, and I want to make sure you are aware of the implications. I will compare yours with the same class in "plain old data" version, with no user-defined constructor (yours has two: the value constructor and the default one).

With your version, it's harder for the user to initialise the object only partially (e.g. initialise charge but not light). This is in general good, but note that partial initialization may still happen (LightCalo calo; calo.charge = 40000.0;).
With the POD version, aggregate initialisation can be used (LightCalo lc { 40000.0, 30000.0, 3.5, 2 };) that can omit the last arguments. This may or may not be desired. On the other end, POD have such guarantees (can be copied with a memcpy(), for example) that allow libraries writing them to disk to take shortcuts. I don't know whether ROOT takes advantage of any of the shortcuts.

In general I recommend POD classes, but that opens for partial initialisation.
So it's up to you and how you feel about it.

};
}

#endif
1 change: 1 addition & 0 deletions sbnobj/Common/Reco/classes.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "sbnobj/Common/Reco/VertexHit.h"
#include "sbnobj/Common/Reco/MVAPID.h"
#include "sbnobj/Common/Reco/CRUMBSResult.h"
#include "sbnobj/Common/Reco/LightCalo.h"
#include "sbnobj/Common/Reco/OpT0FinderResult.h"
#include "sbnobj/Common/Reco/CNNScore.h"
#include "sbnobj/Common/Reco/TPCPMTBarycenterMatch.h"
Expand Down
16 changes: 16 additions & 0 deletions sbnobj/Common/Reco/classes_def.xml
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,22 @@
<class name="art::Assns<sbn::crt::CRTHit, recob::Track, void>" />
<class name="art::Wrapper<art::Assns<sbn::crt::CRTHit, recob::Track, void>>" />

<class name="sbn::LightCalo">
</class>
Comment on lines +81 to +82
Copy link
Member

Choose a reason for hiding this comment

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

This needs a checksum and a class version, so that when we change it ROOT knows how to read the old versions. Start with:

Suggested change
<class name="sbn::LightCalo">
</class>
<class name="sbn::LightCalo" classVersion="10" />

then build: it will tell you to build again. Rebuild... at this point you should see a fully successful build. Lastly, commit the new classes_def.xml to the branch.

<class name="std::vector<sbn::LightCalo>" />
<class name="art::Wrapper<sbn::LightCalo>" />
<class name="art::Wrapper<std::vector<sbn::LightCalo>>" />

<class name="art::Assns<recob::Slice, sbn::LightCalo>" />
<class name="art::Wrapper<art::Assns<recob::Slice, sbn::LightCalo>>" />
<class name="art::Assns<sbn::LightCalo, recob::Slice, void>" />
<class name="art::Wrapper<art::Assns<sbn::LightCalo, recob::Slice, void>>"/>

<class name="art::Assns<recob::OpFlash, sbn::LightCalo>" />
<class name="art::Assns<sbn::LightCalo, recob::OpFlash, void>" />
<class name="art::Wrapper<art::Assns<recob::OpFlash, sbn::LightCalo>>" />
<class name="art::Wrapper<art::Assns<sbn::LightCalo, recob::OpFlash, void>>" />
Comment on lines +87 to +95
Copy link
Member

Choose a reason for hiding this comment

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

The void in the associations is default and not necessary, but either way it should be consistent for ease of reading. Also for ease of reading, it would be better to have them in the same order.
Finally, I was under the impression that the explicit registration of the std::pair was needed too (at least it used to be).

Suggested change
<class name="art::Assns<recob::Slice, sbn::LightCalo>" />
<class name="art::Wrapper<art::Assns<recob::Slice, sbn::LightCalo>>" />
<class name="art::Assns<sbn::LightCalo, recob::Slice, void>" />
<class name="art::Wrapper<art::Assns<sbn::LightCalo, recob::Slice, void>>"/>
<class name="art::Assns<recob::OpFlash, sbn::LightCalo>" />
<class name="art::Assns<sbn::LightCalo, recob::OpFlash, void>" />
<class name="art::Wrapper<art::Assns<recob::OpFlash, sbn::LightCalo>>" />
<class name="art::Wrapper<art::Assns<sbn::LightCalo, recob::OpFlash, void>>" />
<class name="std::pair<recob::Slice, sbn::LightCalo>" />
<class name="art::Assns<recob::Slice, sbn::LightCalo>" />
<class name="art::Wrapper<art::Assns<recob::Slice, sbn::LightCalo>>" />
<class name="std::pair<sbn::LightCalo, recob::Slice>" />
<class name="art::Assns<sbn::LightCalo, recob::Slice>" />
<class name="art::Wrapper<art::Assns<sbn::LightCalo, recob::Slice>>"/>
<class name="std::pair<recob::OpFlash, sbn::LightCalo>" />
<class name="art::Assns<recob::OpFlash, sbn::LightCalo>" />
<class name="art::Wrapper<art::Assns<recob::OpFlash, sbn::LightCalo>>" />
<class name="std::pair<sbn::LightCalo, recob::OpFlash>" />
<class name="art::Assns<sbn::LightCalo, recob::OpFlash>" />
<class name="art::Wrapper<art::Assns<sbn::LightCalo, recob::OpFlash>>" />


<class name="sbn::OpT0Finder">
</class>
<class name="std::vector<sbn::OpT0Finder>" />
Expand Down