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

fix bug about lambda error + small cleanup #18896

Merged
merged 1 commit into from
May 27, 2017

Conversation

VinInn
Copy link
Contributor

@VinInn VinInn commented May 23, 2017

@VinInn
Copy link
Contributor Author

VinInn commented May 23, 2017

@arizzi

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

RecoTracker/TkSeedGenerator
RecoTracker/TkTrackingRegions

@perrotta, @cmsbuild, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @makortel, @felicepantaleo, @GiacomoSguazzoni, @rovere, @VinInn, @mschrode, @gpetruc, @ebrondol, @dgulhan this is something you requested to watch as well.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

@VinInn
Copy link
Contributor Author

VinInn commented May 23, 2017

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 23, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/20041/console Started: 2017/05/23 14:59

@VinInn
Copy link
Contributor Author

VinInn commented May 23, 2017

type bug-fix

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18896/20041/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 3717 differences found in the comparisons
  • DQMHistoTests: Total files compared: 24
  • DQMHistoTests: Total histograms compared: 1833058
  • DQMHistoTests: Total failures: 35521
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1797357
  • DQMHistoTests: Total skipped: 180
  • DQMHistoTests: Total Missing objects: 0
  • Checked 98 log files, 14 edm output root files, 24 DQM output files

@perrotta
Copy link
Contributor

Tiny regression in all tracking related quantities, as expected from the description of this PR: this has been verified in some high pt QCD and PU25 TTbar sample. A few exemple follow:

  • Theta of gsf electrons in 400 events produced with wf 10059 (high pt QCD):
    all_testpr18896vsorig_qcd13tevpt3ts3t5in2017wf10059p0c_recogsftracks_electrongsftracks__reco_obj_theta

  • Eta of general tracks in 400 events produced with wf 10059 (high pt QCD):
    all_testpr18896vsorig_qcd13tevpt3ts3t5in2017wf10059p0c_recotracks_generaltracks__reco_obj_eta

  • nSecondaryTracks in recoPFDisplacedVertex in TTbarPUwf25202
    all_testpr18896vsorig_ttbarpuwf25202p0c_recopfdisplacedvertexs_particleflowdisplacedvertex__reco_obj_nsecondarytracks

and so on so forth...

@perrotta
Copy link
Contributor

+1
After visual inspection, #18896 (comment) and #18896 (comment)

@cmsbuild
Copy link
Contributor

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

@slava77
Copy link
Contributor

slava77 commented May 26, 2017 via email

@perrotta
Copy link
Contributor

Ok, but there is anyhow some stat in the forward region in that high pt QCD sample as well, and nothing disrupting shows up there, thus confirming what was observed in the specific plot appended to the description of this PR

Other plots from the 25PU TTbar sample (200 events) do not show any specific accumulation of the tiny regressions in the forward region, though. For example:

  • eta of ak4 PF jets
    all_testpr18896vsorig_ttbarpuwf25202p0c_recopfjets_pfjetsei__reco_obj_eta

  • eta of conversion step tracks
    image

  • eta of general tracks
    image

@slava77
Copy link
Contributor

slava77 commented May 26, 2017 via email

@perrotta
Copy link
Contributor

There are 74 jets with pt>100 and |eta|>1 in the QCD sample, and 73 jets as such in the TTbar sample.
Not such a big stat, but not even fully negligible,..

@slava77
Copy link
Contributor

slava77 commented May 26, 2017 via email

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 205b4ff into cms-sw:master May 27, 2017
@slava77
Copy link
Contributor

slava77 commented May 29, 2017

@VinInn
please clarify which sample you used for the QCD tests in the MTV provided with this PR.
Thank you.

@VinInn
Copy link
Contributor Author

VinInn commented May 29, 2017

the plot refers to 3TeV jets. I also looked to ttbar and 600GeV jets.
One point is that whatever the result one can claim that is intended: jet core is supposed to improve tracking in the core of the jet, not in its periphery.... (and in any case no one has ever tuned (even looked at) jetCore at eta=2 at best of my knowledge)

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.

5 participants