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

implement correct track breaking if propagation fails #8826

Merged
merged 3 commits into from Apr 28, 2015

Conversation

VinInn
Copy link
Contributor

@VinInn VinInn commented Apr 22, 2015

modifications suggested by Ferenc (@sikler) as very visible at low momentum in minimum bias events

The main issue is that previous code in Smoother was "breaking" the track on the "wrong" side
i.e. returning the far part, not the one closest to the vertex.
The new track-shortening algorithm tries to keep the longest possible track preserving the hits closest to the vertex.

Effect is tiny, still visible for 0.5<|eta|<1.2, where more longer tracks are reconstructed.
Small regression expected,

the other two modifications have negligible impact in standard condition.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @VinInn (Vincenzo Innocente) for CMSSW_7_5_X.

implement proper track breaking if propagation fails

It involves the following packages:

RecoTracker/CkfPattern
TrackingTools/TrackFitters

@cmsbuild, @cvuosalo, @nclopezo, @slava77 can you please review it and eventually sign? Thanks.
@ghellwig, @battibass, @makortel, @abbiendi, @GiacomoSguazzoni, @jhgoh, @VinInn, @appeltel, @bellan, @mschrode, @rovere, @gpetruc, @cerati, @trocino, @dgulhan, @venturia this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@nclopezo you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

if(failed) continue;


/* previous code
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove instead of keeping old development "for posterity"

@slava77
Copy link
Contributor

slava77 commented Apr 22, 2015

@cmsbuild please test
@VinInn thanks for the cleanup

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

Pull request #8826 was updated. @cmsbuild, @cvuosalo, @nclopezo, @slava77 can you please check and sign again.

@cmsbuild
Copy link
Contributor

@VinInn VinInn changed the title implement proper track breaking if propagation fails implement correct track breaking if propagation fails Apr 23, 2015
}

// Get inner state
const bool doBackFit = (!doSeedingRegionRebuilding) & (!reverseTrajectories);
Copy link
Contributor

Choose a reason for hiding this comment

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

For clarity, & should be &&, otherwise someone might think bitwise AND is actually needed here instead of logical AND. But this issue doesn't justify changing the PR at this stage -- only fix it if making other changes, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It s intetioanlly BITWISE, because is faster in this particular case.

@cvuosalo
Copy link
Contributor

+1

For #8826 e69d643

A small fix to tracking to reconstruct a few more long tracks.

The code changes are satisfactory, as are the Jenkins tests results. Extended tests against baseline CMSSW_7_5_X_2015-04-20-2300 show no problems but do show small changes to many quantities resulting from the tracking changes.

DQM plots for workflow 25202.0_TTbar_13+TTbar_13 provide an example of the tracking changes, especially in the region 0.5<|eta|<1.2, as expected. This example shows a 2% reduction in the number of mixed triplet seeds.
gentrk2

gentrk3

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_5_X IBs unless changes (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @nclopezo, @ktf, @smuzaffar

@davidlange6
Copy link
Contributor

+1

@slava77
Copy link
Contributor

slava77 commented Apr 29, 2015

There is a pretty visible effect from this change on muon standAloneSET reconstruction (the regular standalone muons are not affected)

QCD dijet 3 TeV (mostly punch-through low-pt muons)
there is some migration from low-hit to higher hit, but the total count decreases by about 5%, which may be real (not just "merging")
all_test8826vsorig_qcd13tevpt3ts3t5wf1313p0c_recotracks_standalonesetmuons_updatedatvtx_reco_obj_found

@jhgoh @battibass @bellan @trocino please follow up.
I'm guessing that SET standalone is using some old PSet configuration which becomes affected by the change here; but it could be some other reason.
You can reprocess with runTheMatrix.py -l 1313 --useInputAll --command=" -n NumberOfEvents"
or just reprocess
/RelValQCD_Pt_3000_3500_13/CMSSW_7_5_0_pre2-MCRUN2_74_V7-v1/GEN-SIM-RECO
My guess is that the effect should also be significant for regular low-pt muons (as from Jpsi or alike)

@slava77
Copy link
Contributor

slava77 commented Apr 29, 2015

Looking at DQM plots, I see that refittedStandAloneMuons also have changes. So, it's probably something to do with the refit method.
For the refitted STA the situation is a bit more clear: increase in the number of hits at the low end
wf1313_refittedsta_recotosimhits

@slava77
Copy link
Contributor

slava77 commented Apr 29, 2015

To come back to the tracker part of the tracking:

regarding the original "Effect is tiny, still visible for 0.5<|eta|<1.2, where more longer tracks are reconstructed."

I think a more visible characterization is that more tracks in this area of |eta| with more lost hits are reconstructed; the number of hits isn't changing noticeably enough.
the following is from 25202 (ttbar with PU35@25ns)
wf25202_general_lostvseta
wf25202_hp_lostvseta

As for the seeds, as Vincenzo suggested over email, indeed, there is an increase/migration of the number of tracks in lower iterations (zero to third; fourth ~same; reduction in the fifth and sixth); which explains the reduction in the number of seeds.

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

Successfully merging this pull request may close these issues.

None yet

5 participants