Skip to content

TrkHitReco: fix defects found by a static sweep of the package #1957

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
oksuzian wants to merge 2 commits into
base: main
Choose a base branch
from
+43 −19
Open

TrkHitReco: fix defects found by a static sweep of the package #1957

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dcb0886
TrkHitReco: fix defects found by a static sweep of the package
oksuzian Aug 29, 2026
43e40f5
fix: propagate TimeDivision instead of asserting it on every ComboHit
oksuzian Aug 31, 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
47 changes: 33 additions & 14 deletions TrkHitReco/src/CombineStrawHits_module.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
#include "TMath.h"

#include <iostream>
#include <numeric>
#include <vector>

namespace mu2e {

Expand Down Expand Up @@ -61,7 +63,10 @@ namespace mu2e {
void produce( art::Event& e);

private:
void combine(EventWindowMarker const& ewm, ComboHitCollection const& chcOrig, ComboHitCollection& chcol);
// origIndex maps a position in chcOrig to the corresponding index in chcol's parent
// collection; it is the identity unless the input was re-sorted first
void combine(EventWindowMarker const& ewm, ComboHitCollection const& chcOrig, ComboHitCollection& chcol,
std::vector<uint16_t> const& origIndex);
void combineHits(const ComboHitCollection& chcOrig, ComboHit& combohit);

int _debug;
Expand Down Expand Up @@ -143,20 +148,29 @@ namespace mu2e {
// select hits based on flag
panels[ch.strawId().uniquePanel()].push_back(ish);
}
// chcolNew's parent is chcH (the unsorted input), so record where each sorted hit came
// from: without this the ComboHits would store positions in chcolsort while resolving
// them against chcOrig, silently corrupting per-hit provenance
std::vector<uint16_t> origIndex;
origIndex.reserve(nsh);
for (uint16_t ipanel=0;ipanel<StrawId::_nupanels;ipanel++){
for (uint16_t ish=0;ish<panels[ipanel].size();ish++){
chcolsort.push_back(chcOrig.at(panels[ipanel][ish]));
origIndex.push_back(panels[ipanel][ish]);
}
}
combine(ewm, chcolsort, *chcolNew);
combine(ewm, chcolsort, *chcolNew, origIndex);
}else{
combine(ewm, chcOrig, *chcolNew);
std::vector<uint16_t> origIndex(chcOrig.size());
std::iota(origIndex.begin(), origIndex.end(), 0);
combine(ewm, chcOrig, *chcolNew, origIndex);
}
event.put(std::move(chcolNew));
}


void CombineStrawHits::combine(EventWindowMarker const& ewm, ComboHitCollection const& chcOrig, ComboHitCollection& chcol)
void CombineStrawHits::combine(EventWindowMarker const& ewm, ComboHitCollection const& chcOrig, ComboHitCollection& chcol,
std::vector<uint16_t> const& origIndex)
{

float minT = _minT;
Expand All @@ -175,7 +189,11 @@ namespace mu2e {
if ( _testflag && hit1.flag().hasAnyProperty(StrawHitFlag::dead)) continue;
if ( _testflag && (!hit1.flag().hasAllProperties(_shsel) || hit1.flag().hasAnyProperty(_shmask))) continue;
ComboHit combohit;
combohit.init(hit1,ich);
combohit.init(hit1,origIndex[ich]);
// TimeDivision is a quality tag: StrawHitRecoUtils sets it only when the longitudinal
// position was actually measured, and leaves it off when StrawResponse had to clamp the
// position to the straw end. It must therefore be propagated from the constituents.
bool tdiv = hit1.flag().hasAllProperties(StrawHitFlag::tdiv);
int panel1 = hit1.strawId().uniquePanel();

for (size_t jch=ich+1;jch<chcOrig.size();++jch) {
Expand All @@ -194,39 +212,40 @@ namespace mu2e {
float wdchi = fabs(hit1.wireDist() - hit2.wireDist())/wderr;
if (wdchi > _maxwdchi) continue;

bool ok = combohit.addIndex(jch);
bool ok = combohit.addIndex(origIndex[jch]);
if (!ok){
std::cout << "CombineStrawHits past limit" << std::endl;
} else {
isUsed[jch]= true;
tdiv &= hit2.flag().hasAllProperties(StrawHitFlag::tdiv);
}
}
// clear the flag bits; they are reset later
const static StrawHitFlag initialFlag("TimeDivision");
combohit._flag = initialFlag;
// clear the flag bits; they are reset below
combohit._flag = StrawHitFlag();
if(tdiv) combohit._flag.merge(StrawHitFlag::tdiv);
int nch = combohit.nCombo();
if(nch < _minN || nch > _maxN){
if(_filter)break;
if(_filter)continue;
} else
combohit._flag.merge(StrawHitFlag::nhitsel);
// actually combine the hits if necessar, and make the cuts
if (nch > 1) combineHits(chcOrig, combohit);

auto time = _useTOT ? combohit.correctedTime() : combohit.time();
if (time < minT || time > maxT ){
if(_filter)break;
if(_filter)continue;
} else
combohit._flag.merge(StrawHitFlag::timesel);

auto energy = combohit.energyDep();
if( energy > _maxE || energy < _minE ) {
if(_filter)break;
if(_filter)continue;
} else
combohit._flag.merge(StrawHitFlag::energysel);

auto r2 = combohit.pos().Perp2();
if( r2 < _minR2 || r2 > _maxR2 ) {
if(_filter)break;
if(_filter)continue;
} else
combohit._flag.merge(StrawHitFlag::radsel);
combohit._mask = _mask;
Expand All @@ -249,7 +268,7 @@ namespace mu2e {
{
size_t index = combohit.index(ich);
if (_debug > 3)std::cout << index << ", ";
if (index > chcOrig.size())
if (index >= chcOrig.size())
throw cet::exception("RECO")<<"mu2e::CombineStrawHits: inconsistent index "<<std::endl;

const ComboHit& ch = chcOrig[index];
Expand Down
2 changes: 1 addition & 1 deletion TrkHitReco/src/StereoLineTest_main.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ int main(int argc, char** argv) {
double dz = deltaz/(npts-1);
std::random_device r;
std::default_random_engine eng(r());
std::uniform_int_distribution<int> phirange(-M_PI,M_PI);
std::uniform_real_distribution<double> phirange(-M_PI,M_PI);
std::normal_distribution urand{0.0, ures};
std::normal_distribution vrand{0.0, vres};

Expand Down
13 changes: 9 additions & 4 deletions TrkHitReco/src/StrawHitReco_module.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// Merged with flag and position creation B. Echenard, CalTech
//
// framework
#include "cetlib_except/exception.h"
#include "art/Framework/Principal/Event.h"
#include "art/Framework/Principal/Handle.h"
#include "Offline/GeometryService/inc/GeomHandle.hh"
Expand Down Expand Up @@ -147,6 +148,9 @@ namespace mu2e {
produces<ComboHitCollection>();
produces<IntensityInfoTrackerHits>();
if (_writesh) produces<StrawHitCollection>();
// flagCrossTalk reads the StrawHitCollection, which is only created when it is written out
if (_flagXT && !_writesh)
throw cet::exception("RECO")<<"mu2e::StrawHitReco: FlagCrossTalk requires WriteStrawHitCollection"<<std::endl;
if (_printLevel > 0) std::cout << "In StrawHitReco constructor " << std::endl;
}

Expand Down Expand Up @@ -213,10 +217,11 @@ namespace mu2e {
_shrUtils.createComboHit(ewm, isd, chCol, shCol, caloClusters, pbtOffset,
digi.strawId(), digi.TDC(), digi.TOT(), pmp,
trackerStatus, srep, tt);
//flag straw and electronic cross-talk
if(_flagXT){
_shrUtils.flagCrossTalk(shCol, chCol);
}
}
//flag straw and electronic cross-talk: this scans the whole collection, so it must run once
//per event, not once per digi
if(_flagXT){
_shrUtils.flagCrossTalk(shCol, chCol);
}
if(_writesh)event.put(std::move(shCol));
intInfo->setNTrackerHits(chCol->size());
Expand Down