-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Making the boosted double SV tagger thread-safe #12680
Making the boosted double SV tagger thread-safe #12680
Conversation
Njettiness is not thread-safe and therefore should not be used as a data member of an ESProducer
A new Pull Request was created by @ferencek (Dinko Ferenček) for CMSSW_8_0_X. It involves the following packages: RecoBTag/SecondaryVertex @cmsbuild, @cvuosalo, @davidlange6, @slava77 can you please review it and eventually sign? Thanks. Following commands in first line of a comment are recognized
|
@cmsbuild please test |
The tests are being triggered in jenkins. |
Just to give some sense of urgency, over 120 workflows are failing in CMSSW_8_0_THREADED_X without the thread-safe fix. |
I am approving it and starting 00h00 IB now |
merge |
…readSafe_from-CMSSW_7_6_1 Making the boosted double SV tagger thread-safe
+1 Urgent bug fix for to prevent multi-threaded jobs from crashing. There should be no change in monitored quantities. #12679 is the 76X version of this PR. The code change is satisfactory, and Jenkins tests against baseline CMSSW_8_0_X_2015-12-04-1100 show no significant differences, as expected. An extended test of workflow 25202.0_TTbar_13 with 70 events against baseline CMSSW_8_0_X_2015-12-04-1100 also shows no significant differences. Timing measurements show no significant difference. A related module even seems to show a tiny timing decrease:
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar |
@Dr15Jones , we now have a new issue in threaded IBs. |
@smuzaffar, can you give an example of a workflow that is failing? |
@ferencek , see the workflows here https://cms-sw.github.io/relvalLogDetail.html#slc6_amd64_gcc493;CMSSW_8_0_THREADED_X_2015-12-09-1100 for example 4.67, 4.68 (failing in step3) |
I think the problem is fastjet::contrib::DefaultMeasure::UpdateAxesFast |
This error is already present in IBs before this PR was merged, for example CMSSW_8_0_THREADED_X_2015-12-04-1100: |
I think I have the needed fix to fastjet-contrib |
@Dr15Jones, thanks for the fix. I wonder why this issue did not show up earlier since there are other places in CMSSW where Njettiness is used, in particular in the MiniAOD production. Also, what is the mechanism to propagate this fix upstream. |
fastjet::contrib::Njettiness
is not thread-safe and therefore should not be used as a data member of an ESProducer. This PR addresses the problem by defining a local instance offastjet::contrib::Njettiness
instead of using a global instance defined as a data member of theCandidateBoostedDoubleSecondaryVertexComputer
class.This PR is urgent since it fixes CMSSW crashes occurring in the multi-threaded jobs.