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

Backward search implementation #345

Merged
merged 7 commits into from
Aug 24, 2021
Merged

Backward search implementation #345

merged 7 commits into from
Aug 24, 2021

Conversation

osschar
Copy link
Collaborator

@osschar osschar commented Aug 18, 2021

  1. Mechanics of backward-search + configs for pixeless and tobtec iterations.
  2. Flexible SteeringParams::iterator that gives more control over layer iteration.

Validation / MTV has been run by Leonardo two weeks back.

CMSSW json files will have to be remade as there is a new member in IterationParams.

At some point we should revisit what propagate-to-PCA does(and when):

  • cmssw is not using it;
  • in standalone it has to be turned on but it happens before backward-search for the two iterations where backwards-search is actually used.

@leonardogiannini
Copy link
Contributor

what about the changes in RecoTracker/MkFit/plugins/createPhase1TrackerGeometry.cc ? are they still necessary for the PR to work?

@osschar
Copy link
Collaborator Author

osschar commented Aug 18, 2021

oh yes, the same ... there is extra stuff in there for bkw-search params since the previous commit

@slava77
Copy link
Collaborator

slava77 commented Aug 27, 2021

do we have validation plots for this PR?
I find some drop in efficiency in the initialStep in the 10mu sample
http://uaf-10.t2.ucsd.edu/~slava77/figs/mic/mtv/10mu_mkFit-pr344-pr345-qCompat_md94a476_c5e29902/plots_building_initialStep/effandfakePtEtaPhi.pdf
(ignore the black histogram there; it's my devel in progress)

@mmasciov
Copy link
Collaborator

@leonardogiannini had tested this PR few weeks ago. @leonardogiannini, could you post a link?

if (do_backward_fit)
{
// a) BackwardFitBH works on MkBuilder::m_tracks
builder.BackwardFitBH();
builder.BackwardFit();
Copy link
Collaborator

Choose a reason for hiding this comment

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

this looks like the place that can change the physics results in the initialStep (the rest looks more related to the backward search, which is not used in the initialStep).

@osschar is my guess correct?
I can make an incremental diff to confirm that some time later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmmh, could be. They used to be the same ... will cross check the code, we can bisect afterwards if I don't find anything.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have a check and a proposal for a "revert" in #349

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants