Skip to content
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
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
6 changes: 6 additions & 0 deletions sbncode/CAFMaker/CAFMakerParams.h
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,12 @@ namespace caf
"crttracks" // sbnd
};

Atom<string> SBNDCRTVetoLabel {
Name("SBNDCRTVetoLabel"),
Comment("Label of sbnd CRT Veto."),
"crtveto" // sbnd
};

Atom<string> SBNDFrameShiftInfoLabel {
Name("SBNDFrameShiftInfoLabel"),
Comment("Label of sbnd frame shift."),
Expand Down
23 changes: 22 additions & 1 deletion sbncode/CAFMaker/CAFMaker_module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,11 @@
#include "sbnobj/Common/Trigger/ExtraTriggerInfo.h"
#include "sbnobj/Common/Reco/CRUMBSResult.h"
#include "sbnobj/Common/Reco/OpT0FinderResult.h"
#include "sbnobj/SBND/CRT/CRTVeto.hh"
#include "sbnobj/Common/Reco/CorrectedOpFlashTiming.h"
#include "sbnobj/SBND/Timing/TimingInfo.hh"
#include "sbnobj/SBND/Timing/FrameShiftInfo.hh"


// GENIE
#include "Framework/EventGen/EventRecord.h"
#include "Framework/Ntuple/NtpMCEventRecord.h"
Expand Down Expand Up @@ -1713,6 +1713,7 @@ void CAFMaker::produce(art::Event& evt) noexcept {
std::vector<caf::SRCRTTrack> srcrttracks;
std::vector<caf::SRCRTSpacePoint> srcrtspacepoints;
std::vector<caf::SRSBNDCRTTrack> srsbndcrttracks;
caf::SRSBNDCRTVeto srsbndcrtveto;
caf::SRSBNDFrameShiftInfo srsbndframeshiftinfo;
caf::SRSBNDTimingInfo srsbndtiminginfo;

Expand Down Expand Up @@ -1783,6 +1784,25 @@ void CAFMaker::produce(art::Event& evt) noexcept {
FillSBNDCRTTrack(sbndcrttracks[i], srsbndcrttracks.back());
}
}

// Fill CRT Veto
art::Handle<std::vector<sbnd::crt::CRTVeto>> sbndcrtveto_handle;
GetByLabelStrict(evt, fParams.SBNDCRTVetoLabel(), sbndcrtveto_handle);
std::vector<art::Ptr<sbnd::crt::CRTVeto>> vetoPtrs;
// fill into event
if (sbndcrtveto_handle.isValid()) {
art::fill_ptr_vector(vetoPtrs, sbndcrtveto_handle);
// Only one valid veto per event
if (vetoPtrs.size() == 1) {
Copy link
Member

Choose a reason for hiding this comment

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

Please do not create art pointers unless they are really necessary.
Here:

Suggested change
std::vector<art::Ptr<sbnd::crt::CRTVeto>> vetoPtrs;
// fill into event
if (sbndcrtveto_handle.isValid()) {
art::fill_ptr_vector(vetoPtrs, sbndcrtveto_handle);
// Only one valid veto per event
if (vetoPtrs.size() == 1) {
// fill into event
if (sbndcrtveto_handle.isValid()) {
// Only one valid veto per event
if (sbndcrtveto_handle->size() == 1) {

and then pass to the function a simple sbnd::crt::CRTVeto const* pointer (&sbndcrtveto_handle->front()) or, better, a reference sbnd::crt::CRTVeto const& (sbndcrtveto_handle->front()).
The reference is "better" than a pointer in that the latter would convey the message that the object could be absent (null pointer), which is not the case in your code.

// And associated SpacePoint objects
art::FindManyP<sbnd::crt::CRTSpacePoint> spAssoc(sbndcrtveto_handle, evt, fParams.SBNDCRTVetoLabel());
if (spAssoc.isValid()) {
// There is one vector of SpacePoints per Veto --> can be empty if no veto condition was satisfied
Copy link
Member

Choose a reason for hiding this comment

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

Across quite a few locations there are still quite a few indentation issues, this probably comes from tab size in the editor you are using - try and untabify to solve.

Copy link
Author

Choose a reason for hiding this comment

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

I think you're right. I see something completely different on my end. The indentation shown here doesn't look like what's shown in my text editor (I've been using vim)

Copy link
Member

Choose a reason for hiding this comment

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

Still needs resolving

const std::vector<art::Ptr<sbnd::crt::CRTSpacePoint>> veto_sp_v(spAssoc.at(vetoPtrs[0].key()));
Copy link
Member

Choose a reason for hiding this comment

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

You are asking the key() of the art pointer created from the first element of a data product. The value of such key is always 0.
Also, avoid copying the vector of space point pointers: use a reference.

Suggested change
const std::vector<art::Ptr<sbnd::crt::CRTSpacePoint>> veto_sp_v(spAssoc.at(vetoPtrs[0].key()));
const std::vector<art::Ptr<sbnd::crt::CRTSpacePoint>>& veto_sp_v(spAssoc.at(0));

FillSBNDCRTVeto(vetoPtrs[0], veto_sp_v, srsbndcrtveto);
}
}
}

art::Handle<sbnd::timing::FrameShiftInfo> sbndframeshiftinfo_handle;
GetByLabelStrict(evt, fParams.SBNDFrameShiftInfoLabel(), sbndframeshiftinfo_handle);
Expand Down Expand Up @@ -2541,6 +2561,7 @@ void CAFMaker::produce(art::Event& evt) noexcept {
rec.ncrt_spacepoints = srcrtspacepoints.size();
rec.sbnd_crt_tracks = srsbndcrttracks;
rec.nsbnd_crt_tracks = srsbndcrttracks.size();
rec.sbnd_crt_veto = srsbndcrtveto;
rec.opflashes = srflashes;
rec.nopflashes = srflashes.size();
rec.sbnd_frames = srsbndframeshiftinfo;
Expand Down
19 changes: 19 additions & 0 deletions sbncode/CAFMaker/FillReco.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,25 @@ namespace caf
srsbndcrttrack.tof = track.ToF();
}

void FillSBNDCRTVeto(const art::Ptr<sbnd::crt::CRTVeto> &veto,
const std::vector<art::Ptr<sbnd::crt::CRTSpacePoint>> &points,
caf::SRSBNDCRTVeto &srsbndcrtveto,
bool allowEmpty)
Copy link
Member

Choose a reason for hiding this comment

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

Indentation

{
srsbndcrtveto.V0 = veto->V0();
srsbndcrtveto.V1 = veto->V1();
srsbndcrtveto.V2 = veto->V2();
srsbndcrtveto.V3 = veto->V3();
srsbndcrtveto.V4 = veto->V4();

// add the CRTSpacePoint associations to the SR Veto
for(auto const& sp : points) {
srsbndcrtveto.sp_position.emplace_back(sp->X(), sp->Y(), sp->Z());
srsbndcrtveto.sp_time.emplace_back(sp->Ts0()); /// ns for SBND CRT SpacePoints
srsbndcrtveto.sp_pe.emplace_back(sp->PE());
Comment on lines +205 to +206
Copy link
Member

Choose a reason for hiding this comment

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

I suggest the use of good old push_back() when the purpose is just to copy the argument into the vector. emplace_back() works too, but it has the slightly different purpose of creating a new object into the vector rather than copying an existing one.

Suggested change
srsbndcrtveto.sp_time.emplace_back(sp->Ts0()); /// ns for SBND CRT SpacePoints
srsbndcrtveto.sp_pe.emplace_back(sp->PE());
srsbndcrtveto.sp_time.push_back(sp->Ts0()); /// ns for SBND CRT SpacePoints
srsbndcrtveto.sp_pe.push_back(sp->PE());

}
}

void FillSBNDFrameShiftInfo(const sbnd::timing::FrameShiftInfo &frame,
caf::SRSBNDFrameShiftInfo &srsbndframe,
bool allowEmpty)
Expand Down
6 changes: 6 additions & 0 deletions sbncode/CAFMaker/FillReco.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
#include "sbnobj/Common/CRT/CRTTrack.hh"
#include "sbnobj/SBND/CRT/CRTSpacePoint.hh"
#include "sbnobj/SBND/CRT/CRTTrack.hh"
#include "sbnobj/SBND/CRT/CRTVeto.hh"
#include "sbnobj/Common/CRT/CRTPMTMatching.hh"
#include "sbnobj/Common/CRT/CRTHitT0TaggingInfo.hh"
#include "sbnobj/Common/PMT/Data/PMTBeamSignal.hh"
Expand Down Expand Up @@ -277,6 +278,11 @@ namespace caf
void FillSBNDCRTTrack(const sbnd::crt::CRTTrack &track,
caf::SRSBNDCRTTrack &srsbndcrttrack,
bool allowEmpty = false);

void FillSBNDCRTVeto(const art::Ptr<sbnd::crt::CRTVeto> &veto,
const std::vector<art::Ptr<sbnd::crt::CRTSpacePoint>> &points,
caf::SRSBNDCRTVeto &srsbndcrtveto,
bool allowEmpty = false);

void FillICARUSOpFlash(const recob::OpFlash &flash,
std::vector<recob::OpHit const*> const& hits,
Expand Down