Skip to content

Commit

Permalink
Merge pull request #37146 from trackreco/from-CMSSW_12_3_X_2022-02-28…
Browse files Browse the repository at this point in the history
…-1100_mkFit_X

[MkFit] Fix UBSAN error - skip processing of layers with no hits.
  • Loading branch information
cmsbuild committed Mar 11, 2022
2 parents 44e0ea5 + b548ebf commit 3faf37c
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 21 deletions.
4 changes: 4 additions & 0 deletions RecoTracker/MkFit/plugins/MkFitProducer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ void MkFitProducer::produce(edm::StreamID iID, edm::Event& iEvent, const edm::Ev
const auto& stripHits = iEvent.get(stripHitsToken_);
const auto& eventOfHits = iEvent.get(eventOfHitsToken_);
const auto& seeds = iEvent.get(seedToken_);
if (seeds.seeds().empty()) {
iEvent.emplace(putToken_, mkfit::TrackVec(), not backwardFitInCMSSW_);
return;
}
// This producer does not strictly speaking need the MkFitGeometry,
// but the ESProducer sets global variables (yes, that "feature"
// should be removed), so getting the MkFitGeometry makes it
Expand Down
10 changes: 8 additions & 2 deletions RecoTracker/MkFitCMS/standalone/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,19 @@ LIB_CMS := ../libMicCMS.so
MAIN := ../mkFit
WRMEMF := ../writeMemoryFile

TGTS := ${LIB_CMS} ${MAIN} ${WRMEMF}
TGTS := ${LIB_CMS} ${MAIN}
ifdef WITH_ROOT
TGTS += ${WRMEMF}
endif

.PHONY: all clean distclean

all: ${TGTS}

SRCS := $(wildcard ${CMS_DIR}/src/*.cc) $(wildcard ${SACMS}/*.cc) ${SACMS}/tkNtuple/WriteMemoryFile.cc
SRCS := $(wildcard ${CMS_DIR}/src/*.cc) $(wildcard ${SACMS}/*.cc)
ifdef WITH_ROOT
SRCS += ${SACMS}/tkNtuple/WriteMemoryFile.cc
endif
SRCB := $(notdir ${SRCS})
DEPS := $(SRCB:.cc=.d)
OBJS := $(SRCB:.cc=.o)
Expand Down
6 changes: 3 additions & 3 deletions RecoTracker/MkFitCore/src/MatriplexPackers.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace mkfit {
int m_pos;

public:
MatriplexPackerSlurpIn(const D& base) : m_base(&base), m_pos(0) {}
MatriplexPackerSlurpIn(const D* base) : m_base(base), m_pos(0) {}

void reset() { m_pos = 0; }

Expand Down Expand Up @@ -69,8 +69,8 @@ namespace mkfit {
int m_off_param;

public:
MatriplexErrParPackerSlurpIn(const T& t)
: MatriplexPackerSlurpIn<D>(*t.errArray()), m_off_param(t.posArray() - this->m_base) {}
MatriplexErrParPackerSlurpIn(const T* t)
: MatriplexPackerSlurpIn<D>(t ? t->errArray() : nullptr), m_off_param(t ? (t->posArray() - this->m_base) : 0) {}

void addInput(const T& item) {
// Could issue L1 prefetch requests here.
Expand Down
13 changes: 7 additions & 6 deletions RecoTracker/MkFitCore/src/MkBuilder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ namespace mkfit {

void MkBuilder::find_tracks_load_seeds_BH(const TrackVec &in_seeds) {
// bool debug = true;

assert(!in_seeds.empty());
m_tracks.reserve(in_seeds.size());
m_tracks.clear();

Expand Down Expand Up @@ -512,10 +512,10 @@ namespace mkfit {
m_job->m_iter_config.m_layer_configs[curr_layer],
m_job->get_mask_for_layer(curr_layer));

dprint("at layer " << curr_layer);
const LayerOfHits &layer_of_hits = m_job->m_event_of_hits[curr_layer];
const LayerInfo &layer_info = trk_info.layer(curr_layer);
const FindingFoos &fnd_foos = FindingFoos::get_finding_foos(layer_info.is_barrel());
dprint("at layer " << curr_layer << ", nHits in layer " << layer_of_hits.nHits());

// Pick up seeds that become active on current layer -- unless already fully loaded.
if (curr_tridx < rng.n_proc()) {
Expand Down Expand Up @@ -595,9 +595,8 @@ namespace mkfit {

void MkBuilder::find_tracks_load_seeds(const TrackVec &in_seeds) {
// This will sort seeds according to iteration configuration.

// m_tracks can be used for BkFit.
m_tracks.clear();
assert(!in_seeds.empty());
m_tracks.clear(); // m_tracks can be used for BkFit.

m_event_of_comb_cands.reset((int)in_seeds.size(), m_job->max_max_cands());

Expand Down Expand Up @@ -800,6 +799,8 @@ namespace mkfit {
layer_plan_it.is_pickup_only(),
iteration_dir);

dprintf(" Number of candidates to process: %d, nHits in layer: %d\n", theEndCand, layer_of_hits.nHits());

if (layer_plan_it.is_pickup_only() || theEndCand == 0)
continue;

Expand Down Expand Up @@ -1002,7 +1003,7 @@ namespace mkfit {
const int theEndCand = find_tracks_unroll_candidates(
seed_cand_idx, start_seed, end_seed, curr_layer, prev_layer, pickup_only, iteration_dir);

dprintf(" Number of candidates to process: %d\n", theEndCand);
dprintf(" Number of candidates to process: %d, nHits in layer: %d\n", theEndCand, layer_of_hits.nHits());

// Don't bother messing with the clone engine if there are no candidates
// (actually it crashes, so this protection is needed).
Expand Down
10 changes: 5 additions & 5 deletions RecoTracker/MkFitCore/src/MkFinder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ namespace mkfit {
void MkFinder::addBestHit(const LayerOfHits &layer_of_hits, const int N_proc, const FindingFoos &fnd_foos) {
// debug = true;

MatriplexHitPacker mhp(*layer_of_hits.hitArray());
MatriplexHitPacker mhp(layer_of_hits.hitArray());

float minChi2[NN];
int bestHit[NN];
Expand Down Expand Up @@ -926,7 +926,7 @@ namespace mkfit {
const FindingFoos &fnd_foos) {
// bool debug = true;

MatriplexHitPacker mhp(*layer_of_hits.hitArray());
MatriplexHitPacker mhp(layer_of_hits.hitArray());

int maxSize = 0;

Expand Down Expand Up @@ -1141,7 +1141,7 @@ namespace mkfit {
const FindingFoos &fnd_foos) {
// bool debug = true;

MatriplexHitPacker mhp(*layer_of_hits.hitArray());
MatriplexHitPacker mhp(layer_of_hits.hitArray());

int maxSize = 0;

Expand Down Expand Up @@ -1358,7 +1358,7 @@ namespace mkfit {
void MkFinder::bkFitInputTracks(TrackVec &cands, int beg, int end) {
// Uses HitOnTrack vector from Track directly + a local cursor array to current hit.

MatriplexTrackPacker mtp(cands[beg]);
MatriplexTrackPacker mtp(&cands[beg]);

int itrack = 0;

Expand All @@ -1385,7 +1385,7 @@ namespace mkfit {
// XXXX - shall we assume only TrackCand-zero is needed and that we can freely
// bork the HoTNode array?

MatriplexTrackPacker mtp(eocss[beg][0]);
MatriplexTrackPacker mtp(&eocss[beg][0]);

int itrack = 0;

Expand Down
10 changes: 5 additions & 5 deletions RecoTracker/MkFitCore/src/MkFitter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ namespace mkfit {
// This might not be true for the last chunk!
// assert(end - beg == NN);

MatriplexTrackPacker mtp(tracks[beg]);
MatriplexTrackPacker mtp(&tracks[beg]);

for (int i = beg; i < end; ++i) {
int itrack = i - beg;
Expand All @@ -147,7 +147,7 @@ namespace mkfit {

// CopyIn seems fast enough, but indirections are quite slow.
for (int hi = 0; hi < m_Nhits; ++hi) {
MatriplexHitPacker mhp(layerHits[hi][0]);
MatriplexHitPacker mhp(layerHits[hi].data());

for (int i = beg; i < end; ++i) {
const int hidx = tracks[i].getHitIdx(hi);
Expand Down Expand Up @@ -276,8 +276,8 @@ namespace mkfit {
// XXXX MT Here the same idx array WAS used for slurping in of tracks and
// hots. With this, two index arrays are built, one within each packer.

MatriplexTrackPacker mtp(tracks[beg]);
MatriplexHoTPacker mhotp(*tracks[beg].getHitsOnTrackArray());
MatriplexTrackPacker mtp(&tracks[beg]);
MatriplexHoTPacker mhotp(tracks[beg].getHitsOnTrackArray());

int itrack = 0;

Expand Down Expand Up @@ -310,7 +310,7 @@ namespace mkfit {

for (int ii = 0; ii < m_Nhits; ++ii) {
// XXXX Assuming hit index corresponds to layer!
MatriplexHitPacker mhp(layersohits[ii][0]);
MatriplexHitPacker mhp(layersohits[ii].data());

for (int i = 0; i < N_proc; ++i) {
const int hidx = m_HoTArr[ii](i, 0, 0).index;
Expand Down

0 comments on commit 3faf37c

Please sign in to comment.