Fixes to PackedCandidate

[makortel]

This PR contains the following fixes to certain issues I encountered when developing #12405

• Add missing +dphi_ to dxy(Point) and dz(Point)
  • Otherwise they give different results than dxy() and dzAssociatedPV() for the point of associated PV
  • Note that this does not affect packing, only unpacking
• Fix smallest negative being packed as smallest positive
  • Currently negative numbers whose abs(pack(x)) < 1 (e.g. (-exp(lmin)) get packed as 0, which is unpacked as the smallest positive numbers (exp(lmin))
  • Here the issue is fixed by packing such negative numbers as -1
  • It follows that for negative numbers the smallest unpacked value (by abs) is -exp(lmin + 1/base*(lmax-lmin)), while for positive numbers the smallest value is exp(lmin)
• Use doubles in intermediate steps in liblogintpack
  • With floats current pack8logCeil() rounds towards zero for certain input values

The PR also adds some unit tests for liblogintpack.

Tested in CMSSW_8_0_X_2015-11-22-1100. The latter two issues were rather rare (handful of PackedCandidates in 9k event RelVal) that they're probably not visible in PR tests, but the first one should show up.

@arizzi @gpetruc

[cmsbuild]

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

[VinInn]

@cmsbuild, please test

[makortel]

Let me know if any of these are wanted for 76X (for the MiniAOD v2).

[cmsbuild]

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

[slava77]

@makortel
please make a PR for 76X as well.
The review will continue here and we will be able to decide if to pick it up based on a working and available PR when the 76X release is made.

[cmsbuild]

+1
Tested at: 34ee08d
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12581/9972/summary.html

[cmsbuild]

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

[makortel]

The 76X-backport is #12583.

[cvuosalo]

@makortel: The Jenkins tests have found a few differences. I want to check that they are what you would expect.
First, from workflow 1330.0:

For the difference between packed and unpacked tracks, two values at -1 are changed by the PR to 0.4 and 0.6. Does this change make sense?

Second, from workflow 25202.0:

If you look carefully, you can see one new bin added at 0.02 by this PR. This addition seems strange, because all other values are less than 0. Why would this change occur?

[cvuosalo]

@makortel: Here are some more plots that may be of concern. I ran workflow 25202.0_TTbar_13 with 70 events against baseline CMSSW_8_0_X_2015-11-25-2300. Comparison shows about 250 quantities with tiny, negligible differences, but there are two increases in fakes that could be a worry:

[makortel]

@cvuosalo I was able to reproduce (after dropping -i all from runTheMatrix), details below. In summary, both changes are expected from "Fix smallest negative being packed as smallest positive".

1330.0: The migration from "sign-flips" is actually be visible in alternative-comparisons page 287

although in these cases these "sign-flips" are categorized as "underflow" (I'll probably fix the validation code to make "sign-flips" more important than under/overflow, or create "sign-flip" category within under/overflow).

In 25202.0 the new bin was also "sign-flipped" before. I agree it is a bit strange that the new bin is the only one in the positive side, but it can be explained. The value from track is -4.48931e-08, and in IB the value from PackedCandidate is 4.13994e-08, which is also the smallest packed positive number exp(-17) (demonstrating the subtle bug). With the PR, the value from PackedCandidate is -4.58249e-08, i.e. -exp(-17+1/128*13), the new smallest negative number. The histogram is filled with (PC-track)/track = 0.0207571. Ideally the validation code should categorize this case as "underflow" and not put it in this histogram, but it needs some development to handle the feature added here, i.e. liblogintpack having different smallest values (by absolute value) for positive and negative numbers. (And I think it's better not to do that in this PR).

[makortel]

@cvuosalo I'm really surprised that MTV plots show differences, since changes in PackedCandidates should not affect tracks in any way (or do we have memory corruption or something?).

Anyway, the *dzpv*cut* histograms from MTV are (surprisingly) "unstable". It is on my todo list to investigate these in detail; on a cursory look on the code I haven't spotted any obvious problems. The cumulative nature of these plots has some role (tiny fluctuations cumulate). If there were any change in tracks I would not worry about these plots at all, but given that tracks should not change, I'm a bit puzzled. Could you share the DQM files or the alternative-comparisons pdf?

[cvuosalo]

@makortel: Thanks for investigating these issues. I've sent you a link to the DQM files from my test.
I'm satisfied that all the differences I posted above have been explained.

[cvuosalo]

+1

For #12581 34ee08d

Small fixes for Packed Candidates to cover special cases. #12583 is the 76X version of this PR, and it has already been merged.

The code changes are satisfactory. Jenkins tests and an extended test show some differences, as discussed above, but these differences are either expected or not related to this PR.