HCAL 25ns reco method 2: fix a remaining thread-safe issue

[lihux25]

] Fixed the following issue:
psfPtr_, psFit_x, psFit_y, psFit_erry and cntNANinfit were not thread-safe due to "non-const
global static variable" --> now fixed

[cmsbuild]

A new Pull Request was created by @lihux25 (Hongxuan Liu) for CMSSW_7_3_X.

Fix a remaining thread-safe issue

It involves the following packages:

RecoLocalCalo/HcalRecAlgos

@cmsbuild, @nclopezo, @StoyanStoynev, @slava77 can you please review it and eventually sign? Thanks.
@argiro 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.

[lihux25]

Tested on 25202.0 and can reproduce the same results as before this fix (expected).

[slava77]

Thank you very much for the quick response.

before the cms-bot gets to running the checks, could you please check

SCRAM_IGNORE_PACKAGES="Fireworks/% Utilities/StaticAnalyzers" USER_LLVM_CHECKERS="-enable-checker threadsafety -enable-checker cms -disable-checker cms.FunctionDumper" scram b -k -j 16 checker SCRAM_IGNORE_SUBDIRS=test 2>&1 | tee -a runStaticChecks.log

after it completes, look at the results using the command that's printed out in the end, smth similar to
scan-view ../llvm-analysis/2014-11-.........

[lihux25]

Slava, thank you very much for the very useful command! I tried what you sent to me.
Out-of-box, it reports:

Starting LLVM static checkers src
**** ERROR: Unable to find libUtilitiesStaticAnalyzers.so
gmake: *** [checker] Error 1
gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 2

I have to checkout the Utilities/StaticAnalyzers package to compile it by myself...

Anyway, the test shows no sign of thread-safety issue in the PulseShapeFitOOTPileupCorrection.cc(h) code anymore.

[slava77]

Hi Hongxuan
Thank you for the quick check.
The last time I tried the check it worked for me out of the box, maybe it was an extensive build.
Anyways, it looks like the thread safety problem is solved.

@Dr15Jones
Chris,
could you please check this in your area to see if the crash in the threaded build is gone.

[cmsbuild]

+1
Tested at: e432a3c
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6258/718/summary.html

[cmsbuild]

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

[Dr15Jones]

Neither of the member functions in PulseShapeFitOOTPileupCorrection should be considered const. The reason is both those functions call non-const functions of either psfPtr_ or hybridfitter. The reason I say this is if someone thinks they can use the same instance of PulseShapeFitOOTPileupCorrection between two threads becuase they only call const functions they would be surprised.

As long as different instances of PulseShapeFitOOTPileupCorrection are always used on different threads then it appears the class is now thread friendly.

[slava77]

Do I understand it correctly that the physics output shouldn't change?
Please confirm/clarify.

[lihux25]

Yes. The physics results shouldn't change.

[slava77]

+1

for #6258 e432a3c
tested in CMSSW_7_3_X_2014-11-07-0200 (test area sign453)
there are no differences in output and no visible diffs in technical performance either, all as expected.

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_3_X IBs unless changes (tests are also fine). This pull request will be automatically merged.

[slava77]

@lihux25
please modify the description of the PR to note that this is a change in the HCAL code.

[slava77]

I meant to say "the subject of the PR" (the top first line)