Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
706c4d8
Update SystToolsEventWeight for dependent dials
jedori0228 Jun 2, 2023
f0c3c49
Duplicated declaration of fParameterSet removed
jedori0228 Jun 12, 2023
12aa749
Update logger for SystToolsEventWeight_module.cc
jedori0228 Jun 25, 2023
58c6946
Update SystToolsEventWeight module to treat isCorrection=true paramhe…
jedori0228 Jul 27, 2023
fd58a14
Saving non primaries to nu.prim for G4 study
jedori0228 Feb 15, 2024
43893b9
Update systematicstools/nusystematics versions
jedori0228 Feb 16, 2024
7ba8ce5
Merge branch 'release/SBN2023A_NuMI' into feature/jskim_SystToolsUpda…
jedori0228 Feb 16, 2024
658fb3b
Add the MicroBooNE weight calculator that interfaces with geant4reweight
sjgardiner Jul 25, 2023
1f3c410
Some tweaks to adjust for larsim EventWeight (used by uB) vs.
sjgardiner Jul 25, 2023
38da527
geant4reweight interface updates + generic sbn fcls
mastbaum Jul 26, 2023
ce8b26c
g4reweight bugfix and temporary parameter storage
mastbaum Jul 27, 2023
dc8fffd
Fill true trajectory points.
Jul 28, 2023
53c9ee9
Disable cross-plane stub merging.
Jul 28, 2023
2eebbe0
For cosmics that are crossing the cathode we added T0 information in …
gMoreno05 Sep 19, 2023
5dd9140
exclude region feature
Sep 15, 2023
b7657b3
restructure, skip out of fv hits
Sep 18, 2023
2dbf3b3
Allow time shift in trigger to IFBeam timing. Skip first event in str…
Jul 28, 2023
79920dd
Update sbndata
Oct 22, 2023
782e10c
Include Geant4 libraries for buiding
Oct 22, 2023
2591aa1
Fix ALP mass suppression.
Oct 22, 2023
6b24a02
Merge remote-tracking branch 'remotes/origin/feature/jskim_SystToolsU…
brucehoward-physics Feb 19, 2024
c0ca4a5
Merge remote-tracking branch 'remotes/origin/feature/jskim_SaveNonPri…
brucehoward-physics Feb 19, 2024
79e67c8
Revert "Include Geant4 libraries for buiding"
brucehoward-physics Apr 25, 2024
0a96a7e
Revert "g4reweight bugfix and temporary parameter storage"
brucehoward-physics Apr 25, 2024
b4a7daa
Revert "geant4reweight interface updates + generic sbn fcls"
brucehoward-physics Apr 25, 2024
caf0d38
Revert "Some tweaks to adjust for larsim EventWeight (used by uB) vs."
brucehoward-physics Apr 25, 2024
e94da95
Revert "Add the MicroBooNE weight calculator that interfaces with gea…
brucehoward-physics Apr 25, 2024
2b1442d
Update product_deps
brucehoward-physics Apr 25, 2024
4ac9e90
Revert "For cosmics that are crossing the cathode we added T0 informa…
brucehoward-physics Apr 25, 2024
6d185eb
Merge branch 'release/SBN2023A_NuMI' into feature/howard_forNuMI2023A…
miquelnebot May 8, 2024
54dff1e
last valid point for last segment validity
May 7, 2024
ad8a28f
After talking to Moon and Gianluca, adding Gianluca's suggestion to c…
brucehoward-physics May 8, 2024
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
9 changes: 9 additions & 0 deletions sbncode/CAFMaker/FillTrigger.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@ namespace caf
triggerInfo.global_trigger_det_time = trig.TriggerTime();
double diff_ts = triggerInfo.global_trigger_det_time - triggerInfo.beam_gate_det_time;
triggerInfo.trigger_within_gate = diff_ts;

triggerInfo.prev_global_trigger_time = addltrig_info.previousTriggerTimestamp;
triggerInfo.source_type = (int)addltrig_info.sourceType;
triggerInfo.trigger_type = (int)addltrig_info.triggerType;
Comment on lines +18 to +19
Copy link
Member

Choose a reason for hiding this comment

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

The semantically correct way to convert a trigger bit information into a number is sbn::bits::value():

Suggested change
triggerInfo.source_type = (int)addltrig_info.sourceType;
triggerInfo.trigger_type = (int)addltrig_info.triggerType;
triggerInfo.source_type = value(addltrig_info.sourceType);
triggerInfo.trigger_type = value(addltrig_info.triggerType);

If the compiler knows what addltrig_info.sourceType is, it should also already know what value() functions are and where they are. Try?

triggerInfo.trigger_id = addltrig_info.triggerID;
triggerInfo.gate_id = addltrig_info.gateID;
triggerInfo.trigger_count = addltrig_info.triggerCount;
triggerInfo.gate_count = addltrig_info.gateCount;
triggerInfo.gate_delta = (int)addltrig_info.gateCountFromPreviousTrigger;
}

void FillTriggerMC(double absolute_time, caf::SRTrigger& triggerInfo) {
Expand Down
4 changes: 3 additions & 1 deletion sbncode/CAFMaker/FillTrue.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,9 @@ namespace caf {
for(const caf::SRTrueParticle& part: srparticles){
// save the G4 particles that came from this interaction
if(part.interaction_id == (int)i) {
if(part.start_process == caf::kG4primary) srneutrino.prim.push_back(part);
//if(part.start_process == caf::kG4primary) srneutrino.prim.push_back(part);
// Feb. 15th 2024, Jaesung Kim: Saving non-primaries for G4Reweight study
srneutrino.prim.push_back(part);
Comment on lines +473 to +475
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I approve this change :)

Copy link
Member

Choose a reason for hiding this comment

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

Thank you, @brucehoward-physics.
Isn't this going to add on every neutrino event the whole GEANT4 history of particles, from the lepton the neutrino turned into down to the last electron of a showering pion?


// total up the deposited energy
for(int p = 0; p < 3; ++p) {
Expand Down
35 changes: 35 additions & 0 deletions sbncode/Calibration/TrackCaloSkimmer_module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,41 @@ sbn::TrueParticle TrueParticleInfo(const simb::MCParticle &particle,
}
}

// Save the true trajectory
for (unsigned i_traj = 0; i_traj < particle.NumberTrajectoryPoints(); i_traj++) {
// Get trajectory point
TVector3 traj = particle.Position(i_traj).Vect();
geo::Point_t traj_p(traj.X(), traj.Y(), traj.Z());

// lookup TPC
geo::TPCID tpc; // invalid by default
for (auto const &cryo: geo->Iterate<geo::CryostatGeo>()) {
for (auto const& TPC : geo->Iterate<geo::TPCGeo>(cryo.ID())) {
if (TPC.ActiveBoundingBox().ContainsPosition(traj_p)) {
tpc = TPC.ID();
break;
}
}
if (tpc.isValid) break;
}
Comment on lines +840 to +850
Copy link
Member

Choose a reason for hiding this comment

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

Why not just:

Suggested change
// lookup TPC
geo::TPCID tpc; // invalid by default
for (auto const &cryo: geo->Iterate<geo::CryostatGeo>()) {
for (auto const& TPC : geo->Iterate<geo::TPCGeo>(cryo.ID())) {
if (TPC.ActiveBoundingBox().ContainsPosition(traj_p)) {
tpc = TPC.ID();
break;
}
}
if (tpc.isValid) break;
}
// lookup TPC
geo::TPCID tpc; // invalid by default
for (auto const& TPC : geo->Iterate<geo::TPCGeo>()) {
if (TPC.ActiveBoundingBox().ContainsPosition(traj_p)) {
tpc = TPC.ID();
break;
}
}

or even

Suggested change
// lookup TPC
geo::TPCID tpc; // invalid by default
for (auto const &cryo: geo->Iterate<geo::CryostatGeo>()) {
for (auto const& TPC : geo->Iterate<geo::TPCGeo>(cryo.ID())) {
if (TPC.ActiveBoundingBox().ContainsPosition(traj_p)) {
tpc = TPC.ID();
break;
}
}
if (tpc.isValid) break;
}
// lookup TPC
geo::TPCGeo const* pTPC = geo->PositionToTPCptr(traj_p);
geo::TPCID tpc; // invalid by default
if (pTPC && pTPC->ActiveBoundingBox().ContainsPosition(traj_p)) tpc = pTPC->ID();

However I would suggest caching, since looking for a TPC on every single point is bound to be slow:

Suggested change
// lookup TPC
geo::TPCID tpc; // invalid by default
for (auto const &cryo: geo->Iterate<geo::CryostatGeo>()) {
for (auto const& TPC : geo->Iterate<geo::TPCGeo>(cryo.ID())) {
if (TPC.ActiveBoundingBox().ContainsPosition(traj_p)) {
tpc = TPC.ID();
break;
}
}
if (tpc.isValid) break;
}
// lookup TPC
if (!pLastTPC || !pLastTPC->ContainsPosition(traj_p)) { // TPC changed
pLastTPC = geo->PositionToTPCptr(traj_p);
}
geo::TPCID tpc; // invalid by default
if (pLastTPC && pLastTPC->ActiveBoundingBox().ContainsPosition(traj_p)) tpc = pLastTPC->ID();

with geo::TPCGeo const* pLastTPC = nullptr; defined just before the trajectory point loop. This will execute 2 checks most of the time, while the complete loop above over 8 TPC does on average 4.5.

I know: you are not particularly interested in optimising code which is not yours...


// add in space-charge-deflected position if applicable
geo::Point_t traj_p_sce = tpc.isValid ? TrajectoryToWirePosition(traj_p, tpc) : traj_p;

sbn::Vector3D traj_v;
traj_v.x = traj_p.x();
traj_v.y = traj_p.y();
traj_v.z = traj_p.z();

sbn::Vector3D traj_v_sce;
traj_v_sce.x = traj_p_sce.x();
traj_v_sce.y = traj_p_sce.y();
traj_v_sce.z = traj_p_sce.z();

trueparticle.traj.push_back(traj_v);
trueparticle.traj_sce.push_back(traj_v_sce);
}

return trueparticle;
}

Expand Down
12 changes: 9 additions & 3 deletions sbncode/EventGenerator/MeVPrtl/Tools/ALP/Meson2ALP_tool.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ class Meson2ALP : public IMeVPrtlFlux
double fcG; //!< Axion coupling to gluon
double fcB; //!< Axion coupling to U(1) B boson (before EW symmetry breaking)
double fcW; //!< Axion coupling to SU(2) W bosons (before EW symmetry breaking)
bool fIncludeMassSuppressionFactor; // Whether to include a phenomonological suppression to mixing at high mass
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
bool fIncludeMassSuppressionFactor; // Whether to include a phenomonological suppression to mixing at high mass
bool fIncludeMassSuppressionFactor; // Whether to include a phenomenological suppression to mixing at high mass


// branching ratios
double fPi0BR;
Expand Down Expand Up @@ -115,6 +116,7 @@ void Meson2ALP::configure(fhicl::ParameterSet const &pset)
fMaxWeightPi0 = pset.get<double>("MaxWeightPi0", 1.);
fMaxWeightEta = pset.get<double>("MaxWeightEta", 1.);
fMaxWeightEtaP = pset.get<double>("MaxWeightEtaP", 1.);
fIncludeMassSuppressionFactor = pset.get<bool>("IncludeMassSuppressionFactor", false);

fPi0BR = Pi0BR();
fEtaBR = EtaBR();
Expand All @@ -136,7 +138,7 @@ double Meson2ALP::EtaBR() const {

double mixing_angle = (1. / sqrt(6)) * (fcG * fpion / ffa) * (fM*fM - (4./9.)*pzero_mass*pzero_mass) / (fM*fM - eta_mass*eta_mass);
double qcd_rate_f = 1.;
if (fM > eta_mass) qcd_rate_f = pow(fM/eta_mass, -1.6);
if (fIncludeMassSuppressionFactor && fM > eta_mass) qcd_rate_f = pow(fM/eta_mass, -1.6);
Copy link
Member

Choose a reason for hiding this comment

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

This correction should have been in a function... has that train left already?


return mixing_angle*mixing_angle*qcd_rate_f;
}
Expand All @@ -148,7 +150,7 @@ double Meson2ALP::EtaPBR() const {

double mixing_angle = (1. / sqrt(12)) * (fcG * fpion / ffa) * (fM*fM - (16./9.)*pzero_mass*pzero_mass) / (fM*fM - etap_mass*etap_mass);
double qcd_rate_f = 1.;
if (fM > etap_mass) qcd_rate_f = pow(fM/etap_mass, -1.6);
if (fIncludeMassSuppressionFactor && fM > etap_mass) qcd_rate_f = pow(fM/etap_mass, -1.6);

return mixing_angle*mixing_angle*qcd_rate_f;
}
Expand All @@ -160,7 +162,7 @@ double Meson2ALP::Pi0BR() const {
double mixing_angle = (1./6) * (fcG * fpion / ffa) * fM*fM / (fM*fM - pzero_mass*pzero_mass);

double qcd_rate_f = 1.;
if (fM > pzero_mass) qcd_rate_f = pow(fM/pzero_mass, -1.6);
if (fIncludeMassSuppressionFactor && fM > pzero_mass) qcd_rate_f = pow(fM/pzero_mass, -1.6);

return mixing_angle*mixing_angle*qcd_rate_f;
}
Expand All @@ -177,6 +179,10 @@ bool Meson2ALP::MakeFlux(const simb::MCFlux &flux, evgen::ldm::MeVPrtlFlux &alp,

// Energy is same as for meson (don't worry about momentum conservation)
double alp_energy = meson.mom.E();

// ignore if alp isn't energetic enough
if (alp_energy < fM) return false;

double alp_momentum = sqrt(alp_energy*alp_energy - fM*fM);

// Momentum 4-vector
Expand Down
104 changes: 98 additions & 6 deletions sbncode/LArRecoProducer/LArReco/TrajectoryMCSFitter.cxx
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
#include "TrajectoryMCSFitter.h"
#include "lardataobj/RecoBase/Track.h"
#include "larcorealg/Geometry/geo_vectors_utils.h"
#include "larcore/Geometry/Geometry.h"
#include "larcore/CoreUtils/ServiceUtil.h"
#include "larcorealg/Geometry/GeometryCore.h"
#include "TMatrixDSym.h"
#include "TMatrixDSymEigen.h"

Expand All @@ -15,7 +18,8 @@ recob::MCSFitResult TrajectoryMCSFitter::fitMcs(const recob::TrackTrajectory& tr
vector<size_t> breakpoints;
vector<float> segradlengths;
vector<float> cumseglens;
breakTrajInSegments(traj, breakpoints, segradlengths, cumseglens);
vector<bool> breakpointsgood;
breakTrajInSegments(traj, breakpoints, segradlengths, cumseglens, breakpointsgood);
//
// Fit segment directions, and get 3D angles between them
//
Expand All @@ -28,7 +32,11 @@ recob::MCSFitResult TrajectoryMCSFitter::fitMcs(const recob::TrackTrajectory& tr
if (p>0) {
if (segradlengths[p]<-100. || segradlengths[p-1]<-100.) {
dtheta.push_back(-999.);
} else {
}
if (!breakpointsgood[p] || !breakpointsgood[p-1]) {
dtheta.push_back(-999.);
}
else {
const double cosval = pcdir0.X()*pcdir1.X()+pcdir0.Y()*pcdir1.Y()+pcdir0.Z()*pcdir1.Z();
//assert(std::abs(cosval)<=1);
//units are mrad
Expand All @@ -49,15 +57,21 @@ recob::MCSFitResult TrajectoryMCSFitter::fitMcs(const recob::TrackTrajectory& tr
}
const ScanResult fwdResult = doLikelihoodScan(dtheta, segradlengths, cumLenFwd, true, momDepConst, pid);
const ScanResult bwdResult = doLikelihoodScan(dtheta, segradlengths, cumLenBwd, false, momDepConst, pid);
//
// std::cout << "fwdResult.p=" << fwdResult.p << " fwdResult.pUnc=" << fwdResult.pUnc << " fwdResult.logL=" << fwdResult.logL << std::endl;
return recob::MCSFitResult(pid,
fwdResult.p,fwdResult.pUnc,fwdResult.logL,
bwdResult.p,bwdResult.pUnc,bwdResult.logL,
segradlengths,dtheta);
}

void TrajectoryMCSFitter::breakTrajInSegments(const recob::TrackTrajectory& traj, vector<size_t>& breakpoints, vector<float>& segradlengths, vector<float>& cumseglens) const {
//
void TrajectoryMCSFitter::breakTrajInSegments(const recob::TrackTrajectory& traj, vector<size_t>& breakpoints, vector<float>& segradlengths, vector<float>& cumseglens, vector<bool>& breakpointsgood) const {
// set fiducial volume
std::vector<geo::BoxBoundedGeo> fiducialVolumes;
fiducialVolumes = setFiducialVolumes();
// set excluded volumes
std::vector<geo::BoxBoundedGeo> excludeVolumes;
excludeVolumes = setExcludeVolumes();

const double trajlen = traj.Length();
const int nseg = std::max(minNSegs_,int(trajlen/segLen_));
const double thisSegLen = trajlen/double(nseg);
Expand All @@ -69,6 +83,8 @@ void TrajectoryMCSFitter::breakTrajInSegments(const recob::TrackTrajectory& traj
auto nextValid=traj.FirstValidPoint();
breakpoints.push_back(nextValid);
auto pos0 = traj.LocationAtPoint(nextValid);
bool pos0good = isInVolume(fiducialVolumes, pos0) && !isInVolume(excludeVolumes, pos0);
breakpointsgood.push_back(pos0good);
nextValid = traj.NextValidPoint(nextValid+1);
int npoints = 0;
while (nextValid!=recob::TrackTrajectory::InvalidIndex) {
Expand All @@ -77,7 +93,9 @@ void TrajectoryMCSFitter::breakTrajInSegments(const recob::TrackTrajectory& traj
pos0=pos1;
npoints++;
if (thislen>=thisSegLen) {
bool pos1good = isInVolume(fiducialVolumes, pos1) && !isInVolume(excludeVolumes, pos1);
breakpoints.push_back(nextValid);
breakpointsgood.push_back(pos1good);
if (npoints>=minHitsPerSegment_) segradlengths.push_back(thislen*lar_radl_inv);
else segradlengths.push_back(-999.);
cumseglens.push_back(cumseglens.back()+thislen);
Expand All @@ -88,7 +106,10 @@ void TrajectoryMCSFitter::breakTrajInSegments(const recob::TrackTrajectory& traj
}
//then add last segment
if (thislen>0.) {
auto endpointpos = traj.LocationAtPoint(traj.LastValidPoint());
bool endpointposgood = isInVolume(fiducialVolumes, endpointpos) && !isInVolume(excludeVolumes, endpointpos);
breakpoints.push_back(traj.LastValidPoint()+1);
breakpointsgood.push_back(endpointposgood);
segradlengths.push_back(thislen*lar_radl_inv);
cumseglens.push_back(cumseglens.back()+thislen);
}
Expand Down Expand Up @@ -202,7 +223,6 @@ double TrajectoryMCSFitter::mcsLikelihood(double p, double theta0x, std::vector<
double result = 0;
for (int i = beg; i != end; i+=incr ) {
if (dthetaij[i]<0) {
//cout << "skip segment with too few points" << endl;
continue;
}
//
Expand Down Expand Up @@ -302,3 +322,75 @@ double TrajectoryMCSFitter::GetE(const double initial_E, const double length_tra
}
return current_E;
}

std::vector<geo::BoxBoundedGeo> TrajectoryMCSFitter::setFiducialVolumes() const {
Copy link
Member

Choose a reason for hiding this comment

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

Considering that this method does not set anything (it's const), a name like getFiducialVolumes() seems more appropriate.

std::vector<geo::BoxBoundedGeo> fiducialVolumes;
std::vector<std::vector<geo::BoxBoundedGeo>> TPCVolumes;
std::vector<geo::BoxBoundedGeo> ActiveVolumes;

const geo::GeometryCore *geometry = lar::providerFrom<geo::Geometry>();
for (auto const &cryoID: geometry->Iterate<geo::CryostatID>()) {
std::vector<geo::BoxBoundedGeo> thisTPCVolumes;
for (auto const& tpc: geometry->Iterate<geo::TPCGeo>(cryoID)) {
thisTPCVolumes.push_back(tpc.ActiveBoundingBox());
}
TPCVolumes.push_back(std::move(thisTPCVolumes));
}
for (const std::vector<geo::BoxBoundedGeo> &tpcs: TPCVolumes) {
double xMin = std::min_element(tpcs.begin(), tpcs.end(), [](auto &lhs, auto &rhs) { return lhs.MinX() < rhs.MinX(); })->MinX();
double xMax = std::max_element(tpcs.begin(), tpcs.end(), [](auto &lhs, auto &rhs) { return lhs.MaxX() < rhs.MaxX(); })->MaxX();
double yMin = std::min_element(tpcs.begin(), tpcs.end(), [](auto &lhs, auto &rhs) { return lhs.MinY() < rhs.MinY(); })->MinY();
double yMax = std::max_element(tpcs.begin(), tpcs.end(), [](auto &lhs, auto &rhs) { return lhs.MaxY() < rhs.MaxY(); })->MaxY();
double zMin = std::min_element(tpcs.begin(), tpcs.end(), [](auto &lhs, auto &rhs) { return lhs.MinZ() < rhs.MinZ(); })->MinZ();
double zMax = std::max_element(tpcs.begin(), tpcs.end(), [](auto &lhs, auto &rhs) { return lhs.MaxZ() < rhs.MaxZ(); })->MaxZ();
ActiveVolumes.emplace_back(xMin, xMax, yMin, yMax, zMin, zMax);
}
Comment on lines +332 to +347
Copy link
Member

Choose a reason for hiding this comment

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

Matter of which one is more readable/understandable:

Suggested change
for (auto const &cryoID: geometry->Iterate<geo::CryostatID>()) {
std::vector<geo::BoxBoundedGeo> thisTPCVolumes;
for (auto const& tpc: geometry->Iterate<geo::TPCGeo>(cryoID)) {
thisTPCVolumes.push_back(tpc.ActiveBoundingBox());
}
TPCVolumes.push_back(std::move(thisTPCVolumes));
}
for (const std::vector<geo::BoxBoundedGeo> &tpcs: TPCVolumes) {
double xMin = std::min_element(tpcs.begin(), tpcs.end(), [](auto &lhs, auto &rhs) { return lhs.MinX() < rhs.MinX(); })->MinX();
double xMax = std::max_element(tpcs.begin(), tpcs.end(), [](auto &lhs, auto &rhs) { return lhs.MaxX() < rhs.MaxX(); })->MaxX();
double yMin = std::min_element(tpcs.begin(), tpcs.end(), [](auto &lhs, auto &rhs) { return lhs.MinY() < rhs.MinY(); })->MinY();
double yMax = std::max_element(tpcs.begin(), tpcs.end(), [](auto &lhs, auto &rhs) { return lhs.MaxY() < rhs.MaxY(); })->MaxY();
double zMin = std::min_element(tpcs.begin(), tpcs.end(), [](auto &lhs, auto &rhs) { return lhs.MinZ() < rhs.MinZ(); })->MinZ();
double zMax = std::max_element(tpcs.begin(), tpcs.end(), [](auto &lhs, auto &rhs) { return lhs.MaxZ() < rhs.MaxZ(); })->MaxZ();
ActiveVolumes.emplace_back(xMin, xMax, yMin, yMax, zMin, zMax);
}
for (auto const &cryoID: geometry->Iterate<geo::CryostatID>()) {
auto [ beginTPC, endTPC ] = geometry->Iterate<geo::TPCGeo>(cryoID);
geo::BoxBoundedGeo activeVolume = iTPC->ActiveBoundingBox();
while (++iTPC != endTPC)
activeVolume.ExtendToInclude(iTPC->ActiveBoundingBox());
ActiveVolumes.push_back(activeVolume);
}

double fidInsetMinX = 0;
double fidInsetMaxX = 0;
double fidInsetMinY = 0;
double fidInsetMaxY = 0;
double fidInsetMinZ = 0;
double fidInsetMaxZ = 0;

if (fiducialVolumeInsets_.size() == 6) {
fidInsetMinX = fiducialVolumeInsets_[0];
fidInsetMaxX = fiducialVolumeInsets_[1];
fidInsetMinY = fiducialVolumeInsets_[2];
fidInsetMaxY = fiducialVolumeInsets_[3];
fidInsetMinZ = fiducialVolumeInsets_[4];
fidInsetMaxZ = fiducialVolumeInsets_[5];
}
else if (!fiducialVolumeInsets_.empty()) {
std::cout << "Error: fiducialVolumeInsets vector must have length of 6, not fiducializing" << std::endl;
}
for (const geo::BoxBoundedGeo &AV: ActiveVolumes) {
fiducialVolumes.emplace_back(AV.MinX() + fidInsetMinX, AV.MaxX() - fidInsetMaxX,
AV.MinY() + fidInsetMinY, AV.MaxY() - fidInsetMaxY,
AV.MinZ() + fidInsetMinZ, AV.MaxZ() - fidInsetMaxZ);
}
return fiducialVolumes;
}

std::vector<geo::BoxBoundedGeo> TrajectoryMCSFitter::setExcludeVolumes() const {
std::vector<geo::BoxBoundedGeo> excludeVolumes;
if (excludeVolumes_.size()%6 != 0) {
std::cout << "Error: excludeVolumes vector must have length multiple of 6, not excluding any regions" << std::endl;
excludeVolumes.emplace_back(-9999.0, -9999.0, -9999.0, -9999.0, -9999.0, -9999.0);
return excludeVolumes;
}
for (unsigned int i=0; i<excludeVolumes_.size()/6; i++) {
excludeVolumes.emplace_back(excludeVolumes_[6*i+0], excludeVolumes_[6*i+1], excludeVolumes_[6*i+2], excludeVolumes_[6*i+3], excludeVolumes_[6*i+4], excludeVolumes_[6*i+5]);
}
Comment on lines +381 to +383
Copy link
Member

Choose a reason for hiding this comment

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

Or:

Suggested change
for (unsigned int i=0; i<excludeVolumes_.size()/6; i++) {
excludeVolumes.emplace_back(excludeVolumes_[6*i+0], excludeVolumes_[6*i+1], excludeVolumes_[6*i+2], excludeVolumes_[6*i+3], excludeVolumes_[6*i+4], excludeVolumes_[6*i+5]);
}
for (unsigned int i=0; i<excludeVolumes_.size(); i+=6) {
excludeVolumes.emplace_back(excludeVolumes_[i+0], excludeVolumes_[i+1], excludeVolumes_[i+2], excludeVolumes_[i+3], excludeVolumes_[i+4], excludeVolumes_[i+5]);
}

return excludeVolumes;
}

bool TrajectoryMCSFitter::isInVolume(const std::vector<geo::BoxBoundedGeo> &volumes, const geo::Point_t &point) const {
for (const geo::BoxBoundedGeo &volume: volumes) {
if (point.X()>=volume.MinX() && point.X()<=volume.MaxX() &&
point.Y()>=volume.MinY() && point.Y()<=volume.MaxY() &&
point.Z()>=volume.MinZ() && point.Z()<=volume.MaxZ()) {
return true;
}
Comment on lines +389 to +393
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
if (point.X()>=volume.MinX() && point.X()<=volume.MaxX() &&
point.Y()>=volume.MinY() && point.Y()<=volume.MaxY() &&
point.Z()>=volume.MinZ() && point.Z()<=volume.MaxZ()) {
return true;
}
if (volume.ContainsPosition(point)) return true;

}
return false;
}
25 changes: 22 additions & 3 deletions sbncode/LArRecoProducer/LArReco/TrajectoryMCSFitter.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,15 @@

#include "fhiclcpp/ParameterSet.h"
#include "fhiclcpp/types/Atom.h"
#include "fhiclcpp/types/Sequence.h"
#include "fhiclcpp/types/Table.h"
#include "canvas/Persistency/Common/Ptr.h"
#include "lardataobj/RecoBase/MCSFitResult.h"
#include "lardataobj/RecoBase/Track.h"
#include "lardata/RecoObjects/TrackState.h"
#include "larcore/Geometry/Geometry.h"
#include "larcore/CoreUtils/ServiceUtil.h"
#include "larcorealg/Geometry/GeometryCore.h"

namespace trkf::sbn {
/**
Expand Down Expand Up @@ -87,10 +91,18 @@ namespace trkf::sbn {
Comment("Angular resolution parameter used in modified Highland formula. Unit is mrad."),
3.0
};
fhicl::Sequence<double> fiducialVolumeInsets {
Name("fiducialVolumeInsets"),
Comment("insets from the TPC boundaries to exclude from the fit")
};
Comment on lines +94 to +97
Copy link
Member

Choose a reason for hiding this comment

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

Since a meaningful value must have 6 elements, we can as well enforce it programmatically:

Suggested change
fhicl::Sequence<double> fiducialVolumeInsets {
Name("fiducialVolumeInsets"),
Comment("insets from the TPC boundaries to exclude from the fit")
};
fhicl::Sequence<double, 6> fiducialVolumeInsets {
Name("fiducialVolumeInsets"),
Comment("insets from the TPC boundaries to exclude from the fit: lower and upper x, then y, then z [cm]"),
std::vector<double>(6, 0.0)
};

The default value adds an implicit no-fiducialisation option.
And a bit of guidance on the format doesn't hurt.

fhicl::Sequence<double> excludeVolumes {
Name("excludeVolumes"),
Comment("regions to exclude")
};
Comment on lines +98 to +101
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 would not be bad to be made optional...

Suggested change
fhicl::Sequence<double> excludeVolumes {
Name("excludeVolumes"),
Comment("regions to exclude")
};
fhicl::Sequence<double> excludeVolumes {
Name("excludeVolumes"),
Comment("box regions to exclude (each is 6 absolute coordinates: lower and upper x, then y, then z) [cm]"),
std::vector<double>{}
};

};
using Parameters = fhicl::Table<Config>;
//
TrajectoryMCSFitter(int pIdHyp, int minNSegs, double segLen, int minHitsPerSegment, int nElossSteps, int eLossMode, double pMin, double pMax, double pStep, double angResol){
TrajectoryMCSFitter(int pIdHyp, int minNSegs, double segLen, int minHitsPerSegment, int nElossSteps, int eLossMode, double pMin, double pMax, double pStep, double angResol, std::vector<double>fiducialVolumeInsets, std::vector<double> excludeVolumes){
pIdHyp_ = pIdHyp;
minNSegs_ = minNSegs;
segLen_ = segLen;
Expand All @@ -101,9 +113,11 @@ namespace trkf::sbn {
pMax_ = pMax;
pStep_ = pStep;
angResol_ = angResol;
fiducialVolumeInsets_ = fiducialVolumeInsets;
excludeVolumes_ = excludeVolumes;
Comment on lines +116 to +117
Copy link
Member

Choose a reason for hiding this comment

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

Small optimisation:

Suggested change
fiducialVolumeInsets_ = fiducialVolumeInsets;
excludeVolumes_ = excludeVolumes;
fiducialVolumeInsets_ = std::move(fiducialVolumeInsets);
excludeVolumes_ = std::move(excludeVolumes);

Otherwise, pass by (constant) reference instead of by value.

}
explicit TrajectoryMCSFitter(const Parameters & p)
: TrajectoryMCSFitter(p().pIdHypothesis(),p().minNumSegments(),p().segmentLength(),p().minHitsPerSegment(),p().nElossSteps(),p().eLossMode(),p().pMin(),p().pMax(),p().pStep(),p().angResol()) {}
: TrajectoryMCSFitter(p().pIdHypothesis(),p().minNumSegments(),p().segmentLength(),p().minHitsPerSegment(),p().nElossSteps(),p().eLossMode(),p().pMin(),p().pMax(),p().pStep(),p().angResol(),p().fiducialVolumeInsets(),p().excludeVolumes()) {}
//
recob::MCSFitResult fitMcs(const recob::TrackTrajectory& traj, bool momDepConst = true) const { return fitMcs(traj,pIdHyp_,momDepConst); }
recob::MCSFitResult fitMcs(const recob::Track& track, bool momDepConst = true) const { return fitMcs(track,pIdHyp_,momDepConst); }
Expand All @@ -117,7 +131,7 @@ namespace trkf::sbn {
return fitMcs(tt,pid,momDepConst);
}
//
void breakTrajInSegments(const recob::TrackTrajectory& traj, std::vector<size_t>& breakpoints, std::vector<float>& segradlengths, std::vector<float>& cumseglens) const;
void breakTrajInSegments(const recob::TrackTrajectory& traj, std::vector<size_t>& breakpoints, std::vector<float>& segradlengths, std::vector<float>& cumseglens, std::vector<bool>& breakpointsgood) const;
void linearRegression(const recob::TrackTrajectory& traj, const size_t firstPoint, const size_t lastPoint, recob::tracking::Vector_t& pcdir) const;
double mcsLikelihood(double p, double theta0x, std::vector<float>& dthetaij, std::vector<float>& seg_nradl, std::vector<float>& cumLen, bool fwd, bool momDepConst, int pid) const;
//
Expand Down Expand Up @@ -146,6 +160,9 @@ namespace trkf::sbn {
double energyLossLandau(const double mass2,const double E2, const double x) const;
//
double GetE(const double initial_E, const double length_travelled, const double mass) const;
std::vector<geo::BoxBoundedGeo> setFiducialVolumes() const;
std::vector<geo::BoxBoundedGeo> setExcludeVolumes() const;
bool isInVolume(const std::vector<geo::BoxBoundedGeo> &volumes, const geo::Point_t &point) const;
//
private:
int pIdHyp_;
Expand All @@ -158,6 +175,8 @@ namespace trkf::sbn {
double pMax_;
double pStep_;
double angResol_;
std::vector<double> fiducialVolumeInsets_;
std::vector<double> excludeVolumes_;
};
}

Expand Down
5 changes: 4 additions & 1 deletion sbncode/LArRecoProducer/mcsproducer.fcl
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
BEGIN_PROLOG
mcs_sbn: {
module_type: MCSFitAllPID
MCS: {}
MCS: {
fiducialVolumeInsets: []
excludeVolumes: []
}
Comment on lines +4 to +7
Copy link
Member

Choose a reason for hiding this comment

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

If the default values above are adopted, this change is not needed.

TrackLabel: pandoraTrack
}

Expand Down
13 changes: 13 additions & 0 deletions sbncode/LArRecoProducer/mcsproducer_icarus.fcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
BEGIN_PROLOG
mcs_sbn: {
module_type: MCSFitAllPID
MCS: {
eLossMode: 2
angResol: 10.0
fiducialVolumeInsets: [10.0, 10.0, 10.0, 10.0, 10.0, 10.0]
excludeVolumes: [190.0, 240.0, -9999.0, 9999.0, -9999.0, 9999.0]
Copy link
Member

Choose a reason for hiding this comment

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

Add a comment: why do we want to remove half a metre roughly around west cryostat cathode?

}
TrackLabel: pandoraTrack
}

END_PROLOG
Loading