Helix

[VinInn]

Speed ups and cleanup in Helix computation
In particular the iteration code in for ArbitraryPlaneCrossing has been "linearized"
The maximum number of iterations reduced to 20 (for good solution never seen exceeding 7)
log info degraded to log debug

15% speedup of Analytical propagator

Regression:
As usual, when touching arithmetic code regression are observed.
Conversions are, as usual, affected more than others...
Fully consistent with what already observed when changes occur in optimization by the compiler

[cmsbuild]

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

Helix

It involves the following packages:

TrackingTools/GeomPropagators

@nclopezo, @cmsbuild, @anton-a, @thspeer, @slava77, @Degano can you please review it and eventually sign? Thanks.
@gpetruc, @cerati, @GiacomoSguazzoni, @rovere, @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.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

[cmsbuild]

+1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3295/877/summary.html

[cmsbuild]

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

[slava77]

@lgray @dbenedet @nancymarinelli
I see that the conversions are systematically low (not just randomly shuffled around)
Taking gamma 35 sample as an example (red is new):

• most of the drop is in E/P ~ 0 and for higher radii
  
  
  So, it's unclear to me if the losses are for normal proper conversions or for some randomly matching tracks here.

Daniele and Lindsey, could you please check the changes on your side, before we push this into the release. (maybe there's a better/cleaner sample to look at)

Vincenzo, is there a place to fine-tune the changes so that the conversions are not lost at this level.

[VinInn]

Hi Slava,
thanks for the interesting observation.
Conversion is, historically, very sensitive to numerical stability.
My "benchmark" is for instance this comparison of the same IB gcc481 vs gcc490

for the changes in question:
We can try again with maxIter to 50 for instance to see if anything changes there..
we can also decide to return whatever has been found after maxIter instead of bailing off...
I will run photon35 and try to identify some "interesting" events

[VinInn]

I made some test with workflow 19 photon 35 GeV
indeed the tail for maxiteration is loooong
strange enough if I cut at 20 the maximum for convergence is 14.
if it is 50 the maximum is 44. if 100 is 94...
I have serious doubt about any algorithm that converges so slowly.
In standard events JetHR the maximum observed is 7 when allowed up to 100

[slava77]

Hi Vincenzo, what was your "benchmark"? was it some TTbar PU or something realistically enhanced with real conversions?

[VinInn]

On 14 Apr, 2014, at 2:53 PM, Slava Krutelyov notifications@github.com wrote:

Hi Vincenzo, what was your "benchmark"? was it some TTbar PU or something realistically enhanced with real conversions?

run2012C JetHT..

in any case It is clear that current Conversion code depends on those slowsly converging trajecttories.
It has to be sorted out at a different level (PAG).

Either we leave this PR pending or we simply closing it as potentially impacting critical physics…

v.

[slava77]

I would keep it pending for now, waiting for some feedback from the egamma POG.
It's still unclear to me now if the loss in yields that I reported corresponds to losses in efficiency in a relevant phase space (not just in some bad quality conversions that would be thrown out anyways)

[bendavid]

Hi,
@slava77 can you comment on what exactly is the selection for the conversions in those plots, is it just what is coming out of the default PhotonValidator configuration in the dqm sequence?

For the conversionStepTracks this part of the code is technically maintained by the tracking POG rather than egamma. (And in practice may be maintained by no one.) I wouldn't be surprised to see similar numerical sensitivity in the conversion vertex fitting itself though.

[slava77]

Hi Josh,

My plots were taken directly from the default PhotonValidator (you can guess the meaning of the plots based on the directory and histogram names).

@cerati @rovere
If the tracking POG is responsible for these, we would still need to understand the impact for real conversions .. are there any other tools in addition to the PhotonValidator ?

[bendavid]

Well, it's not clear from the plots that the difference is only coming from the convStepTracks, there may indeed be some contribution from the ConversionProducer as well (which is maintained by egamma).

[slava77]

I don't find any discrepancies in conversionStepTracks in the gamma35
sample.
So, all the discrepancies in the PhotonValidator DQM plots are from
changes after the tracks.

On 4/14/14, 8:52 AM, Josh Bendavid wrote:

Well, it's not clear from the plots that the difference is only coming
from the convStepTracks, there may indeed be some contribution from the
ConversionProducer as well (which is maintained by egamma).

—
Reply to this email directly or view it on GitHub
#3295 (comment).

---

Vyacheslav (Slava) Krutelyov
TAMU: Physics Dept Texas A&M MS4242, College Station, TX 77843-4242
CERN: 42-R-027
AIM/Skype: siava16 googleTalk: slava77@gmail.com
(630) 291-5128 Cell (US) +41 76 275 7116 Cell (CERN)

---

[VinInn]

On 14 Apr, 2014, at 3:56 PM, Slava Krutelyov notifications@github.com wrote:

I don't find any discrepancies in conversionStepTracks in the gamma35
sample.
So, all the discrepancies in the PhotonValidator DQM plots are from
changes after the tracks.
I see all these large iterations coming from
OutInConversionTrackFinder::tracks
which should show up in conversionStepTracks
v.

[bendavid]

No the InOut and OutIn ecal-seeded tracks are separate from the conversionStepTracks, they have their own collections.

[bendavid]

(InOut and OutIn are the ecal-seeded tracks, whereas the conversionStepTracks are the soft conversion partners seeded from existing standard tracks in the tracker)

[VinInn]

Sorry my conclusions were wrong about the number of iterations (they are counted backward)
so the maximum is of course 6!
the reason should be elsewhere...

[VinInn]

I actually discovered that it is pretty common for conversion seed
https://cmssdt.cern.ch/SDT/lxr/source/RecoEgamma/EgammaPhotonAlgos/src/OutInConversionSeedFinder.cc?v=CMSSW_7_1_X_2014-04-12-0200#474
to propagate to the starting plane...
this also happens on some generalTrack in the TrackProducer...
now I do non protect for this explicitly and the propagation may go backward and bail out
I will protect explicitly...
(In my opinion the protection should go high up in the propagator)

[VinInn]

regressions gone

[cmsbuild]

Pull request #3295 was updated. @nclopezo, @cmsbuild, @anton-a, @thspeer, @slava77, @Degano can you please check and sign again.

[slava77]

+1

for #3295 6326b13
tested in CMSSW_7_1_X_2014-04-12-1400 (test area sign342)
regressions are gone, indeed

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes or unless it breaks tests. @nclopezo, @ktf can you please take care of it?

[cmsbuild]

+1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3295/905/summary.html

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

[cmsbuild]

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