From 2559c20c6ed4f12a6aaca5fa2a5642351f93ef78 Mon Sep 17 00:00:00 2001 From: Joost VandeVondele Date: Tue, 4 Dec 2018 21:59:07 +0100 Subject: [PATCH] [cluster] Fix oversight in TT key reuse In the original code, the position key stored in the TT is used to probe&store TT entries after message passing. Since we only store part of the bits in the TT, this leads to incorrect rehashing. This is fixed in this patch storing also the full key in the send buffers, and using that for hashing after message arrival. Short testing with 4 ranks (old vs new) shows this is effective: Score of mpiold vs mpinew: 84 - 275 - 265 [0.347] 624 Elo difference: -109.87 +/- 20.88 --- src/Makefile | 2 +- src/cluster.cpp | 35 +++++++++++++++++++---------------- src/cluster.h | 10 ++++++---- src/tt.h | 1 - 4 files changed, 26 insertions(+), 22 deletions(-) diff --git a/src/Makefile b/src/Makefile index 879d7a1e..bb370780 100644 --- a/src/Makefile +++ b/src/Makefile @@ -357,7 +357,7 @@ ifeq ($(OS), Android) endif ### 3.10 MPI -ifeq ($(CXX),$(filter $(CXX),mpicxx mpic++ mpiCC)) +ifeq ($(CXX),$(filter $(CXX),mpicxx mpic++ mpiCC mpicxx.mpich)) mpi = yes endif diff --git a/src/cluster.cpp b/src/cluster.cpp index 6900844a..85c3ed56 100644 --- a/src/cluster.cpp +++ b/src/cluster.cpp @@ -46,7 +46,7 @@ static MPI_Comm MoveComm = MPI_COMM_NULL; static MPI_Comm StopComm = MPI_COMM_NULL; static MPI_Datatype TTEntryDatatype = MPI_DATATYPE_NULL; -static std::vector TTBuff; +static std::vector TTBuff; static MPI_Op BestMoveOp = MPI_OP_NULL; static MPI_Datatype MIDatatype = MPI_DATATYPE_NULL; @@ -65,14 +65,16 @@ static void BestMove(void* in, void* inout, int* len, MPI_Datatype* datatype) { void init() { int thread_support; - constexpr std::array TTblocklens = {1, 1, 1, 1, 1, 1}; - const std::array TTdisps = {offsetof(TTEntry, key16), - offsetof(TTEntry, move16), - offsetof(TTEntry, value16), - offsetof(TTEntry, eval16), - offsetof(TTEntry, genBound8), - offsetof(TTEntry, depth8)}; - const std::array TTtypes = {MPI_UINT16_T, + constexpr std::array TTblocklens = {1, 1, 1, 1, 1, 1, 1}; + const std::array TTdisps = {offsetof(KeyedTTEntry, first), + offsetof(KeyedTTEntry, second) + offsetof(TTEntry, key16), + offsetof(KeyedTTEntry, second) + offsetof(TTEntry, move16), + offsetof(KeyedTTEntry, second) + offsetof(TTEntry, value16), + offsetof(KeyedTTEntry, second) + offsetof(TTEntry, eval16), + offsetof(KeyedTTEntry, second) + offsetof(TTEntry, genBound8), + offsetof(KeyedTTEntry, second) + offsetof(TTEntry, depth8)}; + const std::array TTtypes = {MPI_UINT64_T, + MPI_UINT16_T, MPI_UINT16_T, MPI_INT16_T, MPI_INT16_T, @@ -95,7 +97,7 @@ void init() { TTBuff.resize(TTSendBufferSize * world_size); - MPI_Type_create_struct(6, TTblocklens.data(), TTdisps.data(), TTtypes.data(), + MPI_Type_create_struct(7, TTblocklens.data(), TTdisps.data(), TTtypes.data(), &TTEntryDatatype); MPI_Type_commit(&TTEntryDatatype); @@ -185,12 +187,13 @@ int rank() { } void save(Thread* thread, TTEntry* tte, - Key k, Value v, Bound b, Depth d, Move m, Value ev, uint8_t g) { - tte->save(k, v, b, d, m, ev, g); + Key k, Value v, Bound b, Depth d, Move m, Value ev) { + tte->save(k, v, b, d, m, ev); + // Try to add to thread's send buffer { std::lock_guard lk(thread->ttBuffer.mutex); - thread->ttBuffer.buffer.replace(*tte); + thread->ttBuffer.buffer.replace(KeyedTTEntry(k,*tte)); } // Communicate on main search thread @@ -208,9 +211,9 @@ void save(Thread* thread, TTEntry* tte, if (flag) { // Save all recieved entries for (auto&& e : TTBuff) { - replace_tte = TT.probe(e.key(), found); - replace_tte->save(e.key(), e.value(), e.bound(), e.depth(), - e.move(), e.eval(), e.gen()); + replace_tte = TT.probe(e.first, found); + replace_tte->save(e.first, e.second.value(), e.second.bound(), e.second.depth(), + e.second.move(), e.second.eval()); } // Reset send buffer diff --git a/src/cluster.h b/src/cluster.h index ea0b1bbf..4502e518 100644 --- a/src/cluster.h +++ b/src/cluster.h @@ -39,17 +39,19 @@ struct MoveInfo { }; #ifdef USE_MPI +using KeyedTTEntry = std::pair; + constexpr std::size_t TTSendBufferSize = 16; -template class TTSendBuffer : public std::array { +template class TTSendBuffer : public std::array { struct Compare { - inline bool operator()(const TTEntry& lhs, const TTEntry& rhs) { - return lhs.depth() > rhs.depth(); + inline bool operator()(const KeyedTTEntry& lhs, const KeyedTTEntry& rhs) { + return lhs.second.depth() > rhs.second.depth(); } }; Compare compare; public: - bool replace(const TTEntry& value) { + bool replace(const KeyedTTEntry& value) { if (compare(value, this->front())) { std::pop_heap(this->begin(), this->end(), compare); this->back() = value; diff --git a/src/tt.h b/src/tt.h index f231c2ea..1ffd02b2 100644 --- a/src/tt.h +++ b/src/tt.h @@ -41,7 +41,6 @@ namespace Cluster { struct TTEntry { - Key key() const { return (Key )(key16) << 48; } Move move() const { return (Move )move16; } Value value() const { return (Value)value16; } Value eval() const { return (Value)eval16; }