Removed use of CLHEP::HepMatrix::determinant

[Dr15Jones]

The determinant calculation of CLHEP::HepMatrix is not thread safe
since it uses a global memory cache. Switching to SMatrix solve the
problem. One complication is HeMatrix starts counting at 1 while
SMatrix starts at 0.

[cmsbuild]

A new Pull Request was created by @Dr15Jones (Chris Jones) for CMSSW_7_4_X.

Removed use of CLHEP::HepMatrix::determinant

It involves the following packages:

RecoMuon/MuonSeedGenerator

@cmsbuild, @cvuosalo, @nclopezo, @slava77 can you please review it and eventually sign? Thanks.
@battibass, @abbiendi, @jhgoh, @bellan, @trocino, @bachtis, @rociovilar 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.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

[Dr15Jones]

I did test running workflow 14.0 with this change and it succeeded. It was on workflow 14.0 which helgrind found the problem originally.

[Dr15Jones]

please test

[cmsbuild]

The tests are being triggered in jenkins.

[cvuosalo]

Starting matrix tests since Jenkins doesn't seem to be giving any results.

[cmsbuild]

+1
Tested at: 57ef14d
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8318/3517/summary.html

[cvuosalo]

In file SETFilter.cc, on lines 616-619, the static analyzer notes that the "newPhi" variable is calculated but never used:

double newPhi = result_phi.first;
if(fabs(newPhi - phi)<0.02){// too close?
newPhi = phi + 0.02;
}

Looking at the code, I wonder whether newPhi should be used in place of "result_phi.first". If it really isn't needed, it should be deleted. This issue could be addressed in this PR or a later one.

[cvuosalo]

+1

For #8318 57ef14d

This PR replaces use of non-thread-safe CLHEP::HepMatrix with a thread-safe matrix class. The new class should duplicate the old functionality, so there should be no change in reco outputs.

The code change is satisfactory, as are the Jenkins test results. Short matrix tests against baseline CMSSW_7_4_X_2015-03-15-0200 show no significant differences, as expected.

[cvuosalo]

+1

Trying again to approve. Maybe the bot will notice this time.

[cmsbuild]

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

[davidlange6]

+1