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 PackedCandidate vertex to post-packed value #12558

Merged
merged 1 commit into from Nov 29, 2015

Conversation

makortel
Copy link
Contributor

I suspect #12367 was not enough to fully restore the 760pre7 behaviour of PackedCandidate packing given my 800per2 validation report
https://hypernews.cern.ch/HyperNews/CMS/get/relval/4324/16.html
Deleting also vertex_ in packBoth() seems to make a difference in the dxy/dz and track reference point histograms to the right direction.

Tested in 800pre2.

@davidlange6 @arizzi @gpetruc

Otherwise it will not be set to the unpacked values in unpackVtx().
@cmsbuild
Copy link
Contributor

A new Pull Request was created by @makortel (Matti Kortelainen) for CMSSW_8_0_X.

It involves the following packages:

DataFormats/PatCandidates

@cvuosalo, @monttj, @cmsbuild, @slava77, @vadler, @davidlange6 can you please review it and eventually sign? Thanks.
@gpetruc this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

Following commands in first line of a comment are recognized

  • +1|approve[d]|sign[ed]: L1/L2's to approve it
  • -1|reject[ed]: L1/L2's to reject it
  • assign <category>[,<category>[,...]]: L1/L2's to request signatures from other categories
  • unassign <category>[,<category>[,...]]: L1/L2's to remove signatures from other categories
  • hold: L1/all L2's/release manager to mark it as on hold
  • unhold: L1/user who put this PR on hold
  • merge: L1/release managers to merge this request
  • [@cmsbuild,] please test: L1/L2 and selected users to start jenkins tests
  • [@cmsbuild,] please test with cms-sw/cmsdist#<PR>: L1/L2 and selected users to start jenkins tests using externals from cmsdist

@slava77
Copy link
Contributor

slava77 commented Nov 25, 2015

@cmsbuild please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/9943/console

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

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

@slava77 there are some missing matrix maps:

  • 1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15

@makortel
Copy link
Contributor Author

By the way, although this fix only affects the "on the fly packed+unpacked" values (i.e. should have no impact on the packed values on the disk), should it be back-ported to 76X for the MiniAOD v2 campaign? I guess the only real effect would be on whether tracking validation reports mention or not the discrepancies fixed here.

@slava77
Copy link
Contributor

slava77 commented Nov 28, 2015

I think this can be backported to 76X as well (similar to #12583 / #12581 )

@slava77
Copy link
Contributor

slava77 commented Nov 28, 2015

+1

for #12558 9762c63

  • code changes are in line with the PR description
  • jenkins tests pass and comparisons with baseline show differences in miniAOD validation plots comparing reco and packed candidate vertex quantities
    this one from 1330.0 looks more clear as a change to the packed.vz to be not exactly the same as the reco.vz, as expected
    wf1330_vz_packeddiff

@makortel
Copy link
Contributor Author

76X backport is #12588.

davidlange6 added a commit that referenced this pull request Nov 29, 2015
Fix PackedCandidate vertex to post-packed value
@davidlange6 davidlange6 merged commit ab45421 into cms-sw:CMSSW_8_0_X Nov 29, 2015
@makortel makortel deleted the fixPackedCandidateVertex branch October 20, 2016 11:49
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

4 participants