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.

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

Jump to bottom

Add beam constraint to extended tracking #75

Merged
merged 5 commits into from
Mar 9, 2021
Merged

Add beam constraint to extended tracking #75

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a0f54c6
add beam spot constraint flag to save new bcon values
cgsavard Feb 23, 2021
7bb3ebb
add check on scaled chi2 value after beam constraint
cgsavard Feb 23, 2021
672fa91
get rid of code duplication
cgsavard Mar 8, 2021
2552848
include comment on kalmanAddBeamConstr_ boolean
cgsavard Mar 8, 2021
9fe542b
take out unecessary function
cgsavard Mar 9, 2021
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
26 changes: 12 additions & 14 deletions L1Trigger/TrackFindingTMTT/interface/KFTrackletTrack.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,12 @@ namespace tmtt {
unsigned int nHelixParam,
unsigned int iPhiSec,
unsigned int iEtaReg,
bool accepted = true)
bool accepted = true,
bool done_bcon = false,
float qOverPt_bcon = 0.,
float d0_bcon = 0.,
float phi0_bcon = 0.,
float chi2rphi_bcon = 0.)
: l1track3D_(l1track3D),
stubs_(stubs),
hitPattern_(hitPattern),
Expand All @@ -56,11 +61,11 @@ namespace tmtt {
tanLambda_(tanLambda),
chi2rphi_(chi2rphi),
chi2rz_(chi2rz),
done_bcon_(false),
qOverPt_bcon_(qOverPt),
d0_bcon_(d0),
phi0_bcon_(phi0),
chi2rphi_bcon_(chi2rphi),
done_bcon_(done_bcon),
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we choose to add the "bcon" arguments to the KFTrackletTrack constructor, then the setBeamConst() function is no longer needed. Or if we choose to keep the setBeamConstr() function, then the "bcon" arguments don't need adding to the constructor.

Copy link
Author

@cgsavard cgsavard Mar 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

What do you prefer? If in the future we just have extended tracking and a longer track word with both the bcon and non-bcon variables, what makes that transition easiest?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Either solution is fine. Choose whichever is easiest for you.

Copy link
Author

Choose a reason for hiding this comment

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

I took out the setBeamConstr() function

qOverPt_bcon_(qOverPt_bcon),
d0_bcon_(d0_bcon),
phi0_bcon_(phi0_bcon),
chi2rphi_bcon_(chi2rphi_bcon),
nHelixParam_(nHelixParam),
iPhiSec_(iPhiSec),
iEtaReg_(iEtaReg),
Expand All @@ -70,14 +75,6 @@ namespace tmtt {
numIterations_(0),
accepted_(accepted) {}

//--- Optionally std::set track helix params & chi2 if beam-spot constraint is used (for 5-parameter fit).
void setBeamConstr(float qOverPt_bcon, float phi0_bcon, float chi2rphi_bcon) {
done_bcon_ = true;
qOverPt_bcon_ = qOverPt_bcon;
d0_bcon_ = 0.0, phi0_bcon_ = phi0_bcon;
chi2rphi_bcon_ = chi2rphi_bcon;
}

//--- Set/get additional info about fitted track that is specific to individual track fit algorithms (KF, LR, chi2)
//--- and is used for debugging/histogramming purposes.

Expand Down Expand Up @@ -131,6 +128,7 @@ namespace tmtt {
float invPt_bcon() const { return std::abs(qOverPt_bcon_); }
float pt_bcon() const { return 1. / (1.0e-6 + this->invPt_bcon()); }
float phi0_bcon() const { return phi0_bcon_; }
float d0_bcon() const { return d0_bcon_; }

// Phi and z coordinates at which track crosses "chosenR" values used by r-phi HT and rapidity sectors respectively.
// (Optionally with beam-spot constraint applied).
Expand Down
11 changes: 9 additions & 2 deletions L1Trigger/TrackFindingTMTT/interface/L1fittedTrack.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,12 @@ namespace tmtt {
~L1fittedTrack() override = default;

//--- Optionally std::set track helix params & chi2 if beam-spot constraint is used (for 5-parameter fit).
void setBeamConstr(float qOverPt_bcon, float phi0_bcon, float chi2rphi_bcon) {
void setBeamConstr(float qOverPt_bcon, float phi0_bcon, float chi2rphi_bcon, bool accepted) {
done_bcon_ = true;
qOverPt_bcon_ = qOverPt_bcon;
d0_bcon_ = 0.0, phi0_bcon_ = phi0_bcon;
chi2rphi_bcon_ = chi2rphi_bcon;
accepted_ = accepted;
}

//--- Set/get additional info about fitted track that is specific to individual track fit algorithms (KF, LR, chi2)
Expand Down Expand Up @@ -161,7 +162,12 @@ namespace tmtt {
nHelixParam(),
iPhiSec(),
iEtaReg(),
accepted());
accepted(),
done_bcon(),
qOverPt_bcon(),
d0_bcon(),
phi0_bcon(),
chi2rphi_bcon());
return trk_;
}

Expand Down Expand Up @@ -247,6 +253,7 @@ namespace tmtt {
return 1. / (small + this->invPt_bcon());
}
float phi0_bcon() const { return phi0_bcon_; }
float d0_bcon() const { return d0_bcon_; }

// Phi and z coordinates at which track crosses "chosenR" values used by r-phi HT and rapidity sectors respectively.
// (Optionally with beam-spot constraint applied).
Expand Down
10 changes: 9 additions & 1 deletion L1Trigger/TrackFindingTMTT/src/KFbase.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,15 @@ namespace tmtt {
if (nHelixPar_ == 5) {
double chi2rphi_bcon = 0.;
TVectorD trackPars_bcon = trackParams_BeamConstr(cand, chi2rphi_bcon);
fitTrk.setBeamConstr(trackPars_bcon[QOVERPT], trackPars_bcon[PHI0], chi2rphi_bcon);

// Check scaled chi2 cut
vector<double> kfLayerVsChiSqCut = settings_->kfLayerVsChiSq5();
double chi2scaled = chi2rphi_bcon / settings_->kalmanChi2RphiScale() + fitTrk.chi2rz();
bool accepted = true;
if (chi2scaled > kfLayerVsChiSqCut[cand->nStubLayers()])
accepted = false;

fitTrk.setBeamConstr(trackPars_bcon[QOVERPT], trackPars_bcon[PHI0], chi2rphi_bcon, accepted);
}
}

Expand Down
1 change: 1 addition & 0 deletions L1Trigger/TrackFindingTMTT/src/Settings.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ namespace tmtt {
//kalmanDebugLevel_(2), // Good for debugging
kalmanMinNumStubs_(4),
kalmanMaxNumStubs_(6),
kalmanAddBeamConstr_(false), // Apply post-fit beam-spot constraint to 5-param fit
kalmanRemove2PScut_(true),
kalmanMaxSkipLayersHard_(1), // On "hard" input tracks
kalmanMaxSkipLayersEasy_(2), // On "easy" input tracks
Expand Down
71 changes: 39 additions & 32 deletions L1Trigger/TrackFindingTracklet/src/HybridFit.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,17 +195,29 @@ void HybridFit::Fit(Tracklet* tracklet, std::vector<const Stub*>& trackstublist)
<< ", phi0 = " << trk.phi0() << ", eta = " << trk.eta() << ", z0 = " << trk.z0()
<< ", chi2 = " << trk.chi2() << ", accepted = " << trk.accepted();

// Tracklet wants phi0 with respect to lower edge of sector, not global phi0.
double phi0fit = reco::reduceRange(trk.phi0() - iSector_ * 2 * M_PI / N_SECTOR + 0.5 * settings_.dphisectorHG());
double d0,chi2rphi,phi0,qoverpt = -999;
if (trk.done_bcon()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you eliminate all the duplicate code between the done_bcon and not done_bcon options? e.g. How about defining float phi0fit, rinvfit, d0fit & chi2rphi, and setting them to bcon or non-bcon values, according to the value of done_bcon. Then these variables can be passed to setFitPars().

Copy link
Author

Choose a reason for hiding this comment

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

I have made changes to reflect this, let me know if this is what you were invisioning.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, almost. Though you should be able to eliminate also duplicate code at https://github.com/cgsavard/cmssw/blob/add_beam_constr/L1Trigger/TrackFindingTracklet/src/HybridFit.cc#L206 and https://github.com/cgsavard/cmssw/blob/add_beam_constr/L1Trigger/TrackFindingTracklet/src/HybridFit.cc#L215 .

Copy link
Author

Choose a reason for hiding this comment

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

Both of these lines rely on the trk phi0 which has a bcon version so this will be different with the beam spot constraint turned on

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't you do:

if (trk.done_bcon()) {
phi0fit = trk.phi0_bcon();
...
} else {
phi0fit = trk.phi0();
...
}
phi0fit = reco::reduceRange(phi0fit - iSector_ * 2 * M_PI / N_SECTOR + 0.5 * settings_.dphisectorHG());
...

Copy link
Author

Choose a reason for hiding this comment

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

I see. In this care then I should also do this to rinvfit I would presume? It would be the same thing with

if (trk.done_bcon()) {
qoverpt = trk.qOverPt_bcon();
...
} else {
qoverpt = trk.qOverPt();
...
}
rinvfit = 0.01 * settings_.c() * settings_.bfield() * qoverpt;
...

Copy link
Author

Choose a reason for hiding this comment

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

I made these changes

d0 = trk.d0_bcon();
chi2rphi = trk.chi2rphi_bcon();
phi0 = trk.phi0_bcon();
qoverpt = trk.qOverPt_bcon();
} else if (!trk.done_bcon()) {
d0 = trk.d0();
chi2rphi = trk.chi2rphi();
phi0 = trk.phi0();
qoverpt = trk.qOverPt();
}

double rinvfit = 0.01 * settings_.c() * settings_.bfield() * trk.qOverPt();
// Tracklet wants phi0 with respect to lower edge of sector, not global phi0.
double phi0fit = reco::reduceRange(phi0 - iSector_ * 2 * M_PI / N_SECTOR + 0.5 * settings_.dphisectorHG());
double rinvfit = 0.01 * settings_.c() * settings_.bfield() * qoverpt;

int irinvfit = rinvfit / settings_.krinvpars();
int iphi0fit = phi0fit / settings_.kphi0pars();
int itanlfit = trk.tanLambda() / settings_.ktpars();
int iz0fit = trk.z0() / settings_.kz0pars();
int id0fit = trk.d0() / settings_.kd0pars();
int ichi2rphifit = trk.chi2rphi() / 16;
int id0fit = d0 / settings_.kd0pars();
int ichi2rphifit = chi2rphi / 16;
int ichi2rzfit = trk.chi2rz() / 16;

const vector<const tmtt::Stub*>& stubsFromFit = trk.stubs();
Expand All @@ -216,34 +228,29 @@ void HybridFit::Fit(Tracklet* tracklet, std::vector<const Stub*>& trackstublist)
l1stubsFromFit.push_back(l1s);
}

if (settings_.printDebugKF()) {
edm::LogVerbatim("L1track") << "#stubs before/after KF fit = " << TMTTstubs.size() << "/"
<< l1stubsFromFit.size();
}

tracklet->setFitPars(rinvfit,
phi0fit,
trk.d0(),
trk.tanLambda(),
trk.z0(),
trk.chi2rphi(),
trk.chi2rz(),
rinvfit,
phi0fit,
trk.d0(),
trk.tanLambda(),
trk.z0(),
trk.chi2rphi(),
trk.chi2rz(),
irinvfit,
iphi0fit,
id0fit,
itanlfit,
iz0fit,
ichi2rphifit,
ichi2rzfit,
trk.hitPattern(),
l1stubsFromFit);
phi0fit,
d0,
trk.tanLambda(),
trk.z0(),
chi2rphi,
trk.chi2rz(),
rinvfit,
phi0fit,
d0,
trk.tanLambda(),
trk.z0(),
chi2rphi,
trk.chi2rz(),
irinvfit,
iphi0fit,
id0fit,
itanlfit,
iz0fit,
ichi2rphifit,
ichi2rzfit,
trk.hitPattern(),
l1stubsFromFit);
} else {
if (settings_.printDebugKF()) {
edm::LogVerbatim("L1track") << "FitTrack:KF rejected track";
Expand Down