Skip to content
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.

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

Update mkFit for CMSSW_12_2_0 #35974

Merged
merged 2 commits into from
Nov 11, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
113 changes: 108 additions & 5 deletions RecoTracker/MkFit/plugins/MkFitIterationConfigESProducer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
namespace {
using namespace mkfit;

void partitionSeeds0(const TrackerInfo &trk_info,
const TrackVec &in_seeds,
const EventOfHits &eoh,
IterationSeedPartition &part) {
[[maybe_unused]] void partitionSeeds0(const TrackerInfo &trk_info,
const TrackVec &in_seeds,
const EventOfHits &eoh,
IterationSeedPartition &part) {
const size_t size = in_seeds.size();

for (size_t i = 0; i < size; ++i) {
Expand Down Expand Up @@ -82,6 +82,109 @@ namespace {
part.m_sort_score[i] = maxEta_regSort * (reg - TrackerInfo::Reg_Barrel) + eta;
}
}

[[maybe_unused]] void partitionSeeds1(const TrackerInfo &trk_info,
const TrackVec &in_seeds,
const EventOfHits &eoh,
IterationSeedPartition &part) {
const LayerInfo &tib1 = trk_info.m_layers[4];
const LayerInfo &tob1 = trk_info.m_layers[10];

const LayerInfo &tidp1 = trk_info.m_layers[21];
const LayerInfo &tidn1 = trk_info.m_layers[48];

const LayerInfo &tecp1 = trk_info.m_layers[27];
const LayerInfo &tecn1 = trk_info.m_layers[54];

// Merge first two layers to account for mono/stereo coverage.
// TrackerInfo could hold joint limits for sub-detectors.
const auto &L = trk_info.m_layers;
const float tidp_rin = std::min(L[21].m_rin, L[22].m_rin);
const float tidp_rout = std::max(L[21].m_rout, L[22].m_rout);
const float tecp_rin = std::min(L[27].m_rin, L[28].m_rin);
const float tecp_rout = std::max(L[27].m_rout, L[28].m_rout);
const float tidn_rin = std::min(L[48].m_rin, L[49].m_rin);
const float tidn_rout = std::max(L[48].m_rout, L[49].m_rout);
const float tecn_rin = std::min(L[54].m_rin, L[55].m_rin);
const float tecn_rout = std::max(L[54].m_rout, L[55].m_rout);
Comment on lines +90 to +109
Copy link
Contributor

Choose a reason for hiding this comment

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

During the review of the Patatrack PRs we received a lot of comments regarding the use of "magic" numbers in the pixel code - even though most of the time we had simply modified or copied existing code.

Could you fix throughout this (and similar files, if any) the use of 4, 10, 21, 48, 27, 54, ... ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fwyzard, thanks for your comment. This has been addressed in #36246.


// Bias towards more aggressive transition-region assignemnts.
// With current tunning it seems to make things a bit worse.
const float tid_z_extra = 0.0f; // 5.0f;
const float tec_z_extra = 0.0f; // 10.0f;

const int size = in_seeds.size();

auto barrel_pos_check = [](const Track &S, float maxR, float rin, float zmax) -> bool {
bool inside = maxR > rin && S.zAtR(rin) < zmax;
return inside;
};

auto barrel_neg_check = [](const Track &S, float maxR, float rin, float zmin) -> bool {
bool inside = maxR > rin && S.zAtR(rin) > zmin;
return inside;
};

auto endcap_pos_check = [](const Track &S, float maxR, float rout, float rin, float zmin) -> bool {
bool inside = maxR > rout ? S.zAtR(rout) > zmin : (maxR > rin && S.zAtR(maxR) > zmin);
return inside;
};

auto endcap_neg_check = [](const Track &S, float maxR, float rout, float rin, float zmax) -> bool {
bool inside = maxR > rout ? S.zAtR(rout) < zmax : (maxR > rin && S.zAtR(maxR) < zmax);
return inside;
};

for (int i = 0; i < size; ++i) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for (int i = 0; i < size; ++i) {
for (size_t i = 0; i < size; ++i) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

const Track &S = in_seeds[i];

HitOnTrack hot = S.getLastHitOnTrack();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
HitOnTrack hot = S.getLastHitOnTrack();
const auto& hot = S.getLastHitOnTrack();

perhaps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

float eta = eoh[hot.layer].GetHit(hot.index).eta();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
float eta = eoh[hot.layer].GetHit(hot.index).eta();
const float eta = eoh[hot.layer].GetHit(hot.index).eta();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


// Region to be defined by propagation / intersection tests
TrackerInfo::EtaRegion reg;

const bool z_dir_pos = S.pz() > 0;
const float maxR = S.maxReachRadius();

if (z_dir_pos) {
bool in_tib = barrel_pos_check(S, maxR, tib1.m_rin, tib1.m_zmax);
bool in_tob = barrel_pos_check(S, maxR, tob1.m_rin, tob1.m_zmax);

if (!in_tib && !in_tob) {
reg = TrackerInfo::Reg_Endcap_Pos;
} else {
bool in_tid = endcap_pos_check(S, maxR, tidp_rout, tidp_rin, tidp1.m_zmin - tid_z_extra);
bool in_tec = endcap_pos_check(S, maxR, tecp_rout, tecp_rin, tecp1.m_zmin - tec_z_extra);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
bool in_tid = endcap_pos_check(S, maxR, tidp_rout, tidp_rin, tidp1.m_zmin - tid_z_extra);
bool in_tec = endcap_pos_check(S, maxR, tecp_rout, tecp_rin, tecp1.m_zmin - tec_z_extra);
const bool in_tid = endcap_pos_check(S, maxR, tidp_rout, tidp_rin, tidp1.m_zmin - tid_z_extra);
const bool in_tec = endcap_pos_check(S, maxR, tecp_rout, tecp_rin, tecp1.m_zmin - tec_z_extra);

also below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


if (!in_tid && !in_tec) {
reg = TrackerInfo::Reg_Barrel;
} else {
reg = TrackerInfo::Reg_Transition_Pos;
}
}
} else {
bool in_tib = barrel_neg_check(S, maxR, tib1.m_rin, tib1.m_zmin);
bool in_tob = barrel_neg_check(S, maxR, tob1.m_rin, tob1.m_zmin);

if (!in_tib && !in_tob) {
reg = TrackerInfo::Reg_Endcap_Neg;
} else {
bool in_tid = endcap_neg_check(S, maxR, tidn_rout, tidn_rin, tidn1.m_zmax + tid_z_extra);
bool in_tec = endcap_neg_check(S, maxR, tecn_rout, tecn_rin, tecn1.m_zmax + tec_z_extra);

if (!in_tid && !in_tec) {
reg = TrackerInfo::Reg_Barrel;
} else {
reg = TrackerInfo::Reg_Transition_Neg;
}
}
}

part.m_region[i] = reg;
part.m_sort_score[i] = 7.0f * (reg - 2) + eta;
Copy link
Contributor

Choose a reason for hiding this comment

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

this "magic formula" may need some comment or explanation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

}
}
} // namespace

class MkFitIterationConfigESProducer : public edm::ESProducer {
Expand Down Expand Up @@ -111,7 +214,7 @@ void MkFitIterationConfigESProducer::fillDescriptions(edm::ConfigurationDescript
std::unique_ptr<mkfit::IterationConfig> MkFitIterationConfigESProducer::produce(
const TrackerRecoGeometryRecord &iRecord) {
auto it_conf = mkfit::ConfigJson_Load_File(configFile_);
it_conf->m_partition_seeds = partitionSeeds0;
it_conf->m_partition_seeds = partitionSeeds1;
return it_conf;
}

Expand Down
30 changes: 23 additions & 7 deletions RecoTracker/MkFit/plugins/MkFitSeedConverter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "DataFormats/TrackerRecHit2D/interface/BaseTrackerRecHit.h"
#include "DataFormats/TrackerRecHit2D/interface/OmniClusterRef.h"
#include "DataFormats/TrackerRecHit2D/interface/trackerHitRTTI.h"
#include "DataFormats/TrackerRecHit2D/interface/SiStripMatchedRecHit2D.h"

#include "DataFormats/TrackerCommon/interface/TrackerTopology.h"
#include "DataFormats/TrackerCommon/interface/TrackerDetSide.h"
Expand Down Expand Up @@ -129,13 +130,28 @@ mkfit::TrackVec MkFitSeedConverter::convertSeeds(const edm::View<TrajectorySeed>
if (not trackerHitRTTI::isFromDet(recHit)) {
throw cms::Exception("Assert") << "Encountered a seed with a hit which is not trackerHitRTTI::isFromDet()";
}
const auto& clusterRef = static_cast<const BaseTrackerRecHit&>(recHit).firstClusterRef();
const auto detId = recHit.geographicalId();
const auto ilay = mkFitGeom.layerNumberConverter().convertLayerNumber(
detId.subdetId(), ttopo.layer(detId), false, ttopo.isStereo(detId), isPlusSide(detId));
LogTrace("MkFitSeedConverter") << " addin hit detid " << detId.rawId() << " index " << clusterRef.index()
<< " ilay " << ilay;
ret.back().addHitIdx(clusterRef.index(), ilay, 0); // per-hit chi2 is not known
auto& baseTrkRecHit = static_cast<const BaseTrackerRecHit&>(recHit);
if (!baseTrkRecHit.isMatched()) {
const auto& clusterRef = baseTrkRecHit.firstClusterRef();
const auto detId = recHit.geographicalId();
const auto ilay = mkFitGeom.layerNumberConverter().convertLayerNumber(
detId.subdetId(), ttopo.layer(detId), false, ttopo.isStereo(detId), isPlusSide(detId));
LogTrace("MkFitSeedConverter") << " adding hit detid " << detId.rawId() << " index " << clusterRef.index()
<< " ilay " << ilay;
ret.back().addHitIdx(clusterRef.index(), ilay, 0); // per-hit chi2 is not known
} else {
auto& matched2D = dynamic_cast<const SiStripMatchedRecHit2D&>(recHit);
const OmniClusterRef* const clRefs[2] = {&matched2D.monoClusterRef(), &matched2D.stereoClusterRef()};
const DetId detIds[2] = {matched2D.monoId(), matched2D.stereoId()};
for (int ii = 0; ii < 2; ++ii) {
const auto& detId = detIds[ii];
const auto ilay = mkFitGeom.layerNumberConverter().convertLayerNumber(
detId.subdetId(), ttopo.layer(detId), false, ttopo.isStereo(detId), isPlusSide(detId));
LogTrace("MkFitSeedConverter") << " adding matched hit detid " << detId.rawId() << " index "
<< clRefs[ii]->index() << " ilay " << ilay;
ret.back().addHitIdx(clRefs[ii]->index(), ilay, 0); // per-hit chi2 is not known
}
}
}
++seed_index;
}
Expand Down