optimize and clean up EventShapeVariables code, add Fox-Wolfram moments

[kpedro88]

I was looking at these variables for an analysis and became dissatisfied with inefficiencies in the code. My changes include:

• remove commented-out code
• update documentation in comments
• use range-based loops
• avoid repeated computations of momentum tensor and eigenvalues
• avoid unnecessary use of TVectorD::operator() (incredibly slow for some reason, thanks ROOT)
  • assignment of ROOT matrix objects is also slow, so tried to avoid that
• avoid repeated trig computations

In addition, @owen234 had introduced the calculation of Fox-Wolfram moments in a private version of this code. I optimized it as follows:

• calculate Legendre polynomials using recursive definition (minimize computation)
• use symmetry factor to reduce summation (i,j term same as j,i term)

The updates are propagated to the associated producer.

attn: @justinrpilot @nstrobbe

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22559/3884

[cmsbuild]

A new Pull Request was created by @kpedro88 (Kevin Pedro) for master.

It involves the following packages:

PhysicsTools/CandAlgos
PhysicsTools/CandUtils

@gpetruc, @cmsbuild, @arizzi, @monttj can you please review it and eventually sign? Thanks.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

[kpedro88]

please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/26745/console Started: 2018/03/09 20:29

[cmsbuild]

+1
Tested at: 188ef26
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-22559/26745/summary.html

[cmsbuild]

Comparison job queued.

[cmsbuild]

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

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 2 differences found in the comparisons
• DQMHistoTests: Total files compared: 29
• DQMHistoTests: Total histograms compared: 2484711
• DQMHistoTests: Total failures: 1
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2484534
• DQMHistoTests: Total skipped: 176
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 1.12 KiB( 23 files compared)
• Checked 118 log files, 9 edm output root files, 29 DQM output files

[kpedro88]

@gpetruc, @arizzi can you sign for analysis?

[gpetruc]

Hi, I think we're supposed to sign only for nanoAOD or analysis releases (but for simplicity they kept the same "analysis" signature for this) Giovanni Il 13 Mar 2018 20:38, "Kevin Pedro" <notifications@github.com> ha scritto:

…

@gpetruc <https://github.com/gpetruc>, @arizzi <https://github.com/arizzi> can you sign for analysis? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#22559 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEbbR-yL6R1fktv7fpMr_jBrR6CqWMHPks5teCAkgaJpZM4Skv5_> .

[kpedro88]

@monttj ping (probably hopeless)

[fabiocos]

@kpedro88 at present the category analysis is practically uncovered, but for the AN branch and NanoAOD where we agreed with @arizzi and @gpetruc to take responsibility.
So effectively the ultimate responsibility is on the release manager

[fabiocos]

@kpedro88 looking at the code the behaviour should be unchanged in the proposed configuration (FW moments are not computed). Where has this been tested?

[kpedro88]

I tested it privately on one of my analysis ntuples (compared distributions before and after, saw they were identical). This code isn't run in a standard sequence in CMSSW, but is used by some people offline (e.g. for the BEST tagger).

[fabiocos]

@kpedro88 thanks, this is way I was asking

[fabiocos]

+1

[fabiocos]

merge