TMVA RReader not multithread safe

Hi,

In our code we are using the RReader class to evaluate a TMVA BDT in an RDF analysis. We noticed however that when running on multiple cores, re-running the same code resulted in slightly different results. In short what we did is:

tmva_reader_1 = std::make_shared<TMVA::Experimental::RReader>(input_files.at(0));
std::vector<Float_t> inputvars {static_cast<Float_t>(mVBF*1000.), 
        static_cast<Float_t>(fabs(dEtaVBF)), 
        static_cast<Float_t>(fwm), 
        static_cast<Float_t>(dRtau0tau1), 
        static_cast<Float_t>(fabs(dPhiVBF)),
        static_cast<Float_t>(mHH*1000), 
        static_cast<Float_t>(VBFeta0eta1)};
tmva_reader_1->Compute(inputvars);

And what we see is the following:

run1:

eventNumber: 174142879
mVBF:        1266.03	1.26603e+06
dEtaVBF:     6.68806	6.68806
fwm:         0.601351	0.601351
dRtau0tau1:  2.33944	2.33944
dPhiVBF:     4.17798	4.17798
mHH:         486.981	486981
VBFeta0eta1: -10.8879	-10.8879
BDT score:   0.426825

run2:

eventNumber: 174142879
mVBF:        1266.03	1.26603e+06
dEtaVBF:     6.68806	6.68806
fwm:         0.601351	0.601351
dRtau0tau1:  2.33944	2.33944
dPhiVBF:     4.17798	4.17798
mHH:         486.981	486981
VBFeta0eta1: -10.8879	-10.8879
BDT score:   -0.91434

We fixed this by adding a lock before the tmva_reader_1, so:

tmva_reader_1 = std::make_shared<TMVA::Experimental::RReader>(input_files.at(0));
std::vector<Float_t> inputvars {static_cast<Float_t>(mVBF*1000.), 
        static_cast<Float_t>(fabs(dEtaVBF)), 
        static_cast<Float_t>(fwm), 
        static_cast<Float_t>(dRtau0tau1), 
        static_cast<Float_t>(fabs(dPhiVBF)),
        static_cast<Float_t>(mHH*1000), 
        static_cast<Float_t>(VBFeta0eta1)};
R__WRITE_LOCKGUARD(ROOT::gCoreMutex);
tmva_reader_1->Compute(inputvars);

We however think that this is an error within the TMVA RReader code itself. Inside the code basically the same lock is introduced, however this is done after assigning the variables of the TMVA Reader class. root/tmva/tmva/inc/TMVA/RReader.hxx at master · root-project/root · GitHub

I suppose simply moving this lock upwards would solve the issue?

Thanks for your feedback.

(might be related to this: Inconsistencies with TMVA model evaluation in pyROOT when using multithreading)

I guess @moneta can help.