diff --git a/PhysicsTools/MVAComputer/BuildFile.xml b/PhysicsTools/MVAComputer/BuildFile.xml index 5058ec02f35a0..b22c2fe08706a 100644 --- a/PhysicsTools/MVAComputer/BuildFile.xml +++ b/PhysicsTools/MVAComputer/BuildFile.xml @@ -5,6 +5,7 @@ + diff --git a/PhysicsTools/MVAComputer/interface/MVAComputer.h b/PhysicsTools/MVAComputer/interface/MVAComputer.h index abb0d3c812c72..0710f6d6ea455 100644 --- a/PhysicsTools/MVAComputer/interface/MVAComputer.h +++ b/PhysicsTools/MVAComputer/interface/MVAComputer.h @@ -139,8 +139,9 @@ class MVAComputer { inline void eval(const VarProcessor *proc, int *outConf, double *output, int *loop, + VarProcessor::LoopCtx& ctx, unsigned int offset, unsigned int out) const - { proc->eval(values_, conf_, output, outConf, loop, offset); } + { proc->eval(values_, conf_, output, outConf, loop, ctx, offset); } inline double output(unsigned int output) const { return values_[conf_[output]]; } @@ -158,7 +159,8 @@ class MVAComputer { DerivContext() : n_(0) {} void eval(const VarProcessor *proc, int *outConf, - double *output, int *loop, + double *output, int *loop, + VarProcessor::LoopCtx& ctx, unsigned int offset, unsigned int out) const; double output(unsigned int output, diff --git a/PhysicsTools/MVAComputer/interface/ProcessRegistry.h b/PhysicsTools/MVAComputer/interface/ProcessRegistry.h index 6255fb71542d0..60155ffab9cd3 100644 --- a/PhysicsTools/MVAComputer/interface/ProcessRegistry.h +++ b/PhysicsTools/MVAComputer/interface/ProcessRegistry.h @@ -18,7 +18,7 @@ #include -#include +#include "tbb/concurrent_unordered_map.h" @@ -92,7 +92,7 @@ namespace PhysicsTools const ProcessRegistry *process); static void unregisterProcess(const char *name); - typedef std::map RegistryMap; + typedef tbb::concurrent_unordered_map RegistryMap; /// return map of all registered processes, allocate if necessary static RegistryMap *getRegistry(); diff --git a/PhysicsTools/MVAComputer/interface/ProcessRegistry.icc b/PhysicsTools/MVAComputer/interface/ProcessRegistry.icc index b2d281ade569c..6416423bf6e95 100644 --- a/PhysicsTools/MVAComputer/interface/ProcessRegistry.icc +++ b/PhysicsTools/MVAComputer/interface/ProcessRegistry.icc @@ -24,10 +24,10 @@ Base_t *ProcessRegistry::Factory::create( { return ProcessRegistry::create(name, calib, parent); } template -std::map*> +tbb::concurrent_unordered_map*> *ProcessRegistry::getRegistry() { - static struct Sentinel { + [[cms::thread_safe]] static struct Sentinel { Sentinel() : instance(new RegistryMap) {} ~Sentinel() { delete instance; instance = 0; } @@ -57,7 +57,7 @@ void ProcessRegistry::registerProcess( template void ProcessRegistry::unregisterProcess( const char *name) -{ RegistryMap *map = getRegistry(); if (map) map->erase(name); } +{ RegistryMap *map = getRegistry(); if (map) map->unsafe_erase(name); } } // namespace PhysicsTools diff --git a/PhysicsTools/MVAComputer/interface/VarProcessor.h b/PhysicsTools/MVAComputer/interface/VarProcessor.h index 24279da25ceaf..eabe2795d32d0 100644 --- a/PhysicsTools/MVAComputer/interface/VarProcessor.h +++ b/PhysicsTools/MVAComputer/interface/VarProcessor.h @@ -86,6 +86,27 @@ class VarProcessor : Context *ctx; }; + + /** \class LoopCtx + * + * \short Hold context information for looping processors + * + * VarProcessor instances which allow looping need to keep + * track of the state of the loop between calls. + * + ************************************************************/ + class LoopCtx { + public: + LoopCtx(): index_(0), offset_(0), size_(0) {} + inline unsigned int& index() { return index_;} + inline unsigned int& offset() { return offset_;} + inline unsigned int& size() { return size_;} + private: + unsigned int index_; + unsigned int offset_; + unsigned int size_ ; + }; + virtual ~VarProcessor(); /// called from the discriminator computer to configure processor @@ -94,22 +115,23 @@ class VarProcessor : /// run the processor evaluation pass on this processor inline void eval(double *input, int *conf, double *output, int *outConf, - int *loop, unsigned int offset) const + int *loop, LoopCtx& loopCtx, unsigned int offset) const { ValueIterator iter(inputVars.iter(), input, conf, - output, outConf, loop, offset); + output, outConf, loop, loopCtx, offset); eval(iter, nInputVars); } /// run the processor evaluation pass on this processor and compute derivatives void deriv(double *input, int *conf, double *output, int *outConf, - int *loop, unsigned int offset, unsigned int in, + int *loop, LoopCtx& ctx, unsigned int offset, unsigned int in, unsigned int out, std::vector &deriv) const; enum LoopStatus { kStop, kNext, kReset, kSkip }; virtual LoopStatus loop(double *output, int *outConf, unsigned int nOutput, + LoopCtx& ctx, unsigned int &nOffset) const { return kStop; } @@ -219,6 +241,8 @@ class VarProcessor : /// test for end of input variable iterator inline operator bool() const { return cur; } + inline LoopCtx& loopCtx() { return ctx;} + /// move to next input variable ValueIterator &operator ++ () { @@ -244,8 +268,8 @@ class VarProcessor : ValueIterator(BitSet::Iterator cur, double *values, int *conf, double *output, int *outConf, - int *loop, unsigned int offset) : - cur(cur), offset(offset), start(values + offset), + int *loop, LoopCtx& ctx, unsigned int offset) : + cur(cur), ctx(ctx), offset(offset), start(values + offset), values(values), conf(conf), loop(loop), output(output + offset), outConf(outConf) { @@ -259,6 +283,7 @@ class VarProcessor : private: BitSet::Iterator cur; + LoopCtx& ctx; const unsigned int offset; double *const start; double *values; diff --git a/PhysicsTools/MVAComputer/src/AtomicId.cc b/PhysicsTools/MVAComputer/src/AtomicId.cc index 6bc73f354a1c7..a6a345391b23e 100644 --- a/PhysicsTools/MVAComputer/src/AtomicId.cc +++ b/PhysicsTools/MVAComputer/src/AtomicId.cc @@ -23,14 +23,12 @@ namespace { // anonymous private: typedef std::multiset IdSet; - IdSet idSet; - static std::allocator stringAllocator; - mutable std::mutex mutex; + IdSet idSet; + std::allocator stringAllocator; + std::mutex mutex; }; } // anonymous namespace -std::allocator IdCache::stringAllocator; - IdCache::~IdCache() { for(std::multiset::iterator iter = diff --git a/PhysicsTools/MVAComputer/src/MVAComputer.cc b/PhysicsTools/MVAComputer/src/MVAComputer.cc index 12077e38eeb07..65d3143da7f3b 100644 --- a/PhysicsTools/MVAComputer/src/MVAComputer.cc +++ b/PhysicsTools/MVAComputer/src/MVAComputer.cc @@ -176,6 +176,7 @@ void MVAComputer::evalInternal(T &ctx) const int *loopOutConf = outConf; int *loopStart = 0; double *loopOutput = output; + VarProcessor::LoopCtx loopCtx; VarProcessor::LoopStatus status = VarProcessor::kNext; unsigned int offset = 0; @@ -192,6 +193,7 @@ void MVAComputer::evalInternal(T &ctx) const if (status != VarProcessor::kSkip) ctx.eval(&*iter->processor, outConf, output, loopStart ? loopStart : loopOutConf, + loopCtx, offset, iter->nOutput); #ifdef DEBUG_EVAL @@ -210,7 +212,7 @@ void MVAComputer::evalInternal(T &ctx) const #endif status = loop->processor->loop(output, outConf, - nextOutput, offset); + nextOutput, loopCtx, offset); if (status == VarProcessor::kReset) { outConf = loopOutConf; @@ -309,10 +311,11 @@ void MVAComputer::writeCalibration(std::ostream &os, void MVAComputer::DerivContext::eval( const VarProcessor *proc, int *outConf, double *output, - int *loop, unsigned int offset, unsigned int out) const + int *loop, VarProcessor::LoopCtx& ctx, + unsigned int offset, unsigned int out) const { proc->deriv(values(), conf(), output, outConf, - loop, offset, n(), out, deriv_); + loop, ctx, offset, n(), out, deriv_); } double MVAComputer::DerivContext::output(unsigned int output, diff --git a/PhysicsTools/MVAComputer/src/ProcForeach.cc b/PhysicsTools/MVAComputer/src/ProcForeach.cc index 52ead5f4da9a0..6ba3d695bb2a5 100644 --- a/PhysicsTools/MVAComputer/src/ProcForeach.cc +++ b/PhysicsTools/MVAComputer/src/ProcForeach.cc @@ -44,6 +44,7 @@ class ProcForeach : public VarProcessor { ValueIterator iter, unsigned int n) const override; virtual LoopStatus loop(double *output, int *conf, unsigned int nOutput, + LoopCtx& ctx, unsigned int &nOffset) const override; private: @@ -56,12 +57,6 @@ class ProcForeach : public VarProcessor { unsigned int count; }; - inline void reset() const { index = offset = size = 0; } - - mutable unsigned int index; - mutable unsigned int offset; - mutable unsigned int size; - unsigned int count; }; @@ -77,7 +72,6 @@ ProcForeach::ProcForeach(const char *name, void ProcForeach::configure(ConfIterator iter, unsigned int n) { - reset(); iter << Variable::FLAG_NONE; while(iter) iter << iter++(Variable::FLAG_MULTIPLE); @@ -104,12 +98,14 @@ ProcForeach::configureLoop(ConfigCtx::Context *ctx_, ConfigCtx::iterator begin, void ProcForeach::eval(ValueIterator iter, unsigned int n) const { + auto const offset = iter.loopCtx().offset(); iter(offset); + auto& loopSize = iter.loopCtx().size(); while(iter) { unsigned int size = iter.size(); - if (!this->size) - this->size = size; + if (!loopSize) + loopSize = size; double value = iter[offset]; iter(value); @@ -120,6 +116,7 @@ void ProcForeach::eval(ValueIterator iter, unsigned int n) const std::vector ProcForeach::deriv( ValueIterator iter, unsigned int n) const { + auto const offset = iter.loopCtx().offset(); std::vector offsets; unsigned int in = 0, out = 0; while(iter) { @@ -137,13 +134,16 @@ std::vector ProcForeach::deriv( VarProcessor::LoopStatus ProcForeach::loop(double *output, int *conf, - unsigned int nOutput, unsigned int &nOffset) const + unsigned int nOutput, LoopCtx& ctx, unsigned int &nOffset) const { + auto& index = ctx.index(); bool endIteration = false; if (index++ == count) { index = 0; endIteration = true; } + auto& offset = ctx.offset(); + auto& size = ctx.size(); if (offset == 0 && !endIteration) { for(int cur = *conf + size; nOutput--; cur += size) @@ -152,7 +152,6 @@ ProcForeach::loop(double *output, int *conf, if (endIteration) { if (++offset >= size) { - reset(); return kStop; } else return kReset; diff --git a/PhysicsTools/MVAComputer/src/VarProcessor.cc b/PhysicsTools/MVAComputer/src/VarProcessor.cc index 9531b93a2b01f..d6373d10862e7 100644 --- a/PhysicsTools/MVAComputer/src/VarProcessor.cc +++ b/PhysicsTools/MVAComputer/src/VarProcessor.cc @@ -119,12 +119,13 @@ VarProcessor *ProcessRegistry &deriv) const { ValueIterator iter(inputVars.iter(), input, conf, - output, outConf, loop, offset); + output, outConf, loop, ctx, offset); eval(iter, nInputVars); diff --git a/PhysicsTools/MVAComputer/test/BuildFile.xml b/PhysicsTools/MVAComputer/test/BuildFile.xml index c8cbc2c1d0dfe..7a9f38797de52 100644 --- a/PhysicsTools/MVAComputer/test/BuildFile.xml +++ b/PhysicsTools/MVAComputer/test/BuildFile.xml @@ -17,3 +17,6 @@ + + + diff --git a/PhysicsTools/MVAComputer/test/testMVAComputer.cppunit.cc b/PhysicsTools/MVAComputer/test/testMVAComputer.cppunit.cc new file mode 100644 index 0000000000000..bb0541b86b18a --- /dev/null +++ b/PhysicsTools/MVAComputer/test/testMVAComputer.cppunit.cc @@ -0,0 +1,242 @@ +// -*- C++ -*- +// +// Package: PhysicsTools/MVAComputer +// Class : testMVAComputer.cppunit +// +// Implementation: +// [Notes on implementation] +// +// Original Author: Christopher Jones +// Created: Fri, 23 Jan 2015 18:54:27 GMT +// + +// system include files + +// user include files +#include + +#include "CondFormats/PhysicsToolsObjects/interface/MVAComputer.h" + +#include "PhysicsTools/MVAComputer/interface/MVAComputer.h" + +class testMVAComputer: public CppUnit::TestFixture +{ + CPPUNIT_TEST_SUITE(testMVAComputer); + + CPPUNIT_TEST(multTest); + CPPUNIT_TEST(optionalTest); + CPPUNIT_TEST(foreachTest); + + CPPUNIT_TEST_SUITE_END(); + +public: + void setUp() {} + void tearDown() {} + + void multTest(); + void optionalTest(); + void foreachTest(); + +}; + +///registration of the test so that the runner can find it +CPPUNIT_TEST_SUITE_REGISTRATION(testMVAComputer); + +using namespace PhysicsTools; + +void +testMVAComputer::multTest() +{ + { + Calibration::MVAComputer calib; + //this will be assigned to 'bit' 0 + calib.inputSet = {Calibration::Variable{AtomicId("x")}}; + //want to read out bit '1' + calib.output = 1; + + // + Calibration::ProcMultiply square; + //we only want to read 1 input + square.in = 1; + //we will read bit '0' and multiply it by itself + square.out = std::vector{{0,0}}; + //input to use comes from bit '0' + square.inputVars.store={0b1}; + //number of bits stored in the last char (?) + square.inputVars.bitsInLast = 1; + + calib.addProcessor(&square); + + MVAComputer mva(&calib,false); + + { + std::vector input; + input.emplace_back("x",2); + + CPPUNIT_ASSERT( 4 == mva.eval(input)); + } + + { + std::vector input; + input.emplace_back("x",3); + + CPPUNIT_ASSERT( 9 == mva.eval(input)); + } + } + + { + Calibration::MVAComputer calib; + //this will be assigned to 'bit' 0 and 1 + calib.inputSet = {Calibration::Variable{AtomicId("x")}, Calibration::Variable{AtomicId("y")} }; + //want to read out bit '1' + calib.output = 2; + + // + Calibration::ProcMultiply square; + //we only want to read 2 input + square.in = 2; + //we will read bit '0' and multiply it by bit '1' + square.out = std::vector{{0,1}}; + //input comes from bits '0' and '1' + square.inputVars.store={0b11}; + //number of bits stored in the last char (?) + square.inputVars.bitsInLast = 2; + + calib.addProcessor(&square); + + MVAComputer mva(&calib,false); + + std::vector input; + input.emplace_back("x",2); + input.emplace_back("y",3); + + CPPUNIT_ASSERT( 6 == mva.eval(input)); + } + +} + +void +testMVAComputer::optionalTest() +{ + { + Calibration::MVAComputer calib; + //this will be assigned to 'bit' 0 + calib.inputSet = {Calibration::Variable{AtomicId("x")}}; + //want to read out bit '1' + calib.output = 1; + + // + Calibration::ProcOptional optional; + //default + optional.neutralPos = {1.}; + //input to use comes from bit '0' + optional.inputVars.store={0b1}; + //number of bits stored in the last char (?) + optional.inputVars.bitsInLast = 1; + + calib.addProcessor(&optional); + + MVAComputer mva(&calib,false); + + { + std::vector input; + input.emplace_back("x",2); + + CPPUNIT_ASSERT( 2 == mva.eval(input)); + } + + { + std::vector input; + + CPPUNIT_ASSERT( 1 == mva.eval(input)); + } + + { + std::vector input; + input.emplace_back("y",2); + + CPPUNIT_ASSERT( 1 == mva.eval(input)); + } + + } +} + +void +testMVAComputer::foreachTest() +{ + { + Calibration::MVAComputer calib; + //this will be assigned to 'bit' 0 + calib.inputSet = {Calibration::Variable{AtomicId("x")}}; + //want to read out bit '2' + calib.output = 6; + + // + Calibration::ProcForeach foreach; + //we only want to read 1 input + foreach.nProcs = 1; + + //input to use comes from bit '0' + foreach.inputVars.store={0b1}; + //number of bits stored in the last char (?) + foreach.inputVars.bitsInLast = 1; + + calib.addProcessor(&foreach); + + + // + Calibration::ProcMultiply square; + //we only want to read 1 input + square.in = 1; + //we will read bit '0' and multiply it by itself + square.out = std::vector{{0,0}}; + //input comes from bits '2' + square.inputVars.store={0b100}; + //number of bits stored in the last char (?) + square.inputVars.bitsInLast = 3; + + calib.addProcessor(&square); + + //Need to break apart the output int different elements + Calibration::ProcSplitter splitter; + splitter.nFirst = 2; + //input comes from bits '3' + splitter.inputVars.store={0b1000}; + //number of bits stored in the last char (?) + splitter.inputVars.bitsInLast = 4; + + calib.addProcessor(&splitter); + + // + Calibration::ProcMultiply join; + //we only want to read 2 inputs + join.in = 2; + //we will read bit '4' and '5' + join.out = std::vector{{0,1}}; + //input comes from bits '4' and '5' + join.inputVars.store={0b110000}; + //number of bits stored in the last char (?) + join.inputVars.bitsInLast = 7; + calib.addProcessor(&join); + + MVAComputer mva(&calib,false); + + { + std::vector input; + input.emplace_back("x",2); + input.emplace_back("x",2); + + CPPUNIT_ASSERT( 4*4 == mva.eval(input)); + } + + { + std::vector input; + input.emplace_back("x",3); + input.emplace_back("x",3); + + CPPUNIT_ASSERT( 9*9 == mva.eval(input)); + } + } +} + +#include