-
Notifications
You must be signed in to change notification settings - Fork 528
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
Certainty propagation negabound [WIP] #487
Changes from 17 commits
54a199a
b6c88dc
db3b419
fbebb38
891ef72
5f8aff0
c664fb5
560e413
4078af0
4fb5522
84113e5
aa266eb
f087312
fdbe61a
ffee98f
4920e74
5e726b4
aba68a7
fec72f0
1b7ac55
9f21d9d
27bdf3a
c206f67
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,3 +8,5 @@ subprojects/* | |
!subprojects/*.wrap | ||
lc0.xcodeproj/ | ||
*.swp | ||
.vs/ | ||
build-cuda.cmd |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,7 +36,7 @@ | |
|
||
namespace lczero { | ||
namespace { | ||
const int kDefaultThreads = 2; | ||
const int kDefaultThreads = 4; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should not change default number of threads without testing as a part of larger change. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe you have a point. If I revert this back to 2 threads, i would like to include a warning though that CP requires more threads especially on fast GPU systems. Maybe in the option descriptions? But thats not to nice either. Ideas? |
||
|
||
const OptionId kThreadsOptionId{"threads", "Threads", | ||
"Number of (CPU) worker threads to use.", 't'}; | ||
|
@@ -98,7 +98,6 @@ const size_t kAvgNodeSize = sizeof(Node) + kAvgMovesPerPosition * sizeof(Edge); | |
const size_t kAvgCacheItemSize = | ||
NNCache::GetItemStructSize() + sizeof(CachedNNRequest) + | ||
sizeof(CachedNNRequest::IdxAndProb) * kAvgMovesPerPosition; | ||
|
||
float ComputeMoveWeight(int ply, float peak, float left_width, | ||
float right_width) { | ||
// Inflection points of the function are at ply = peak +/- width. | ||
|
@@ -154,6 +153,8 @@ void EngineController::PopulateOptions(OptionsParser* options) { | |
defaults->Set<int>(SearchParams::kMaxCollisionEventsId.GetId(), 32); | ||
defaults->Set<int>(SearchParams::kCacheHistoryLengthId.GetId(), 0); | ||
defaults->Set<bool>(SearchParams::kOutOfOrderEvalId.GetId(), true); | ||
defaults->Set<int>(SearchParams::kCertaintyPropagationId.GetId(), 2); | ||
defaults->Set<int>(SearchParams::kCertaintyPropagationDepthId.GetId(), 1); | ||
} | ||
|
||
SearchLimits EngineController::PopulateSearchLimits( | ||
|
@@ -356,6 +357,7 @@ void EngineController::Go(const GoParams& params) { | |
if (info.multipv <= 1) { | ||
ponder_info = info; | ||
if (ponder_info.score) ponder_info.score = -*ponder_info.score; | ||
if (ponder_info.mate) ponder_info.mate = -*ponder_info.mate; | ||
if (ponder_info.depth > 1) ponder_info.depth--; | ||
if (ponder_info.seldepth > 1) ponder_info.seldepth--; | ||
ponder_info.pv.clear(); | ||
|
@@ -465,4 +467,6 @@ void EngineLoop::CmdPonderHit() { engine_.PonderHit(); } | |
|
||
void EngineLoop::CmdStop() { engine_.Stop(); } | ||
|
||
|
||
|
||
} // namespace lczero |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -77,7 +77,8 @@ class EngineController { | |
void PonderHit(); | ||
// Must not block. | ||
void Stop(); | ||
|
||
// Prints verbose move stats of current root | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Period in the end of the sentence. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Deleted the stats command. |
||
|
||
SearchLimits PopulateSearchLimits(int ply, bool is_black, | ||
const GoParams& params, | ||
std::chrono::steady_clock::time_point start_time); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,9 +28,11 @@ | |
#include "mcts/node.h" | ||
|
||
#include <algorithm> | ||
#include <bitset> | ||
#include <cassert> | ||
#include <cmath> | ||
#include <cstring> | ||
#include <iomanip> | ||
#include <iostream> | ||
#include <sstream> | ||
#include <thread> | ||
|
@@ -160,9 +162,58 @@ float Edge::GetP() const { | |
return ret; | ||
} | ||
|
||
void Edge::MakeTerminal(GameResult result) { | ||
certainty_state_ |= kTerminalMask | kCertainMask | kUpperBound | kLowerBound; | ||
certainty_state_ &= kGameResultClear; | ||
if (result == GameResult::WHITE_WON) { | ||
certainty_state_ |= kGameResultWin; | ||
} else if (result == GameResult::BLACK_WON) { | ||
certainty_state_ |= kGameResultLoss; | ||
} | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Run clang-format, it will fix long lines and double empty lines between functions. |
||
void Edge::MakeCertain(GameResult result, CertaintyTrigger trigger) { | ||
certainty_state_ |= kCertainMask | kUpperBound | kLowerBound; | ||
certainty_state_ &= kGameResultClear; | ||
if (result == GameResult::WHITE_WON) { | ||
certainty_state_ |= kGameResultWin; | ||
} else if (result == GameResult::BLACK_WON) { | ||
certainty_state_ |= kGameResultLoss; | ||
} | ||
if (trigger == CertaintyTrigger::TB_HIT) certainty_state_ |= kTBHit; | ||
if (trigger == CertaintyTrigger::TWO_FOLD) certainty_state_ |= kTwoFold; | ||
} | ||
|
||
void Edge::MakeCertain(int q, CertaintyTrigger trigger) { | ||
certainty_state_ |= kCertainMask | kUpperBound | kLowerBound; | ||
certainty_state_ &= kGameResultClear; | ||
if (q == 1) { | ||
certainty_state_ |= kGameResultWin; | ||
} else if (q == -1) { | ||
certainty_state_ |= kGameResultLoss; | ||
} | ||
if (trigger == CertaintyTrigger::TB_HIT) certainty_state_ |= kTBHit; | ||
if (trigger == CertaintyTrigger::TWO_FOLD) certainty_state_ |= kTwoFold; | ||
} | ||
void Edge::SetEQ(int eq) { | ||
certainty_state_ &= kGameResultClear; | ||
if (eq == 1) { | ||
certainty_state_ |= kGameResultWin; | ||
} else if (eq == -1) { | ||
certainty_state_ |= kGameResultLoss; | ||
} | ||
} | ||
|
||
int Edge::GetEQ() const { | ||
if (certainty_state_ & kGameResultLoss) return -1; | ||
if (certainty_state_ & kGameResultWin) return 1; | ||
return 0; | ||
} | ||
|
||
std::string Edge::DebugString() const { | ||
std::ostringstream oss; | ||
oss << "Move: " << move_.as_string() << " p_: " << p_ << " GetP: " << GetP(); | ||
oss << "Move: " << move_.as_string() << " p_: " << p_ << " GetP: " << GetP() | ||
<< " Certainty:" << std::bitset<8>(certainty_state_); | ||
return oss.str(); | ||
} | ||
|
||
|
@@ -197,36 +248,50 @@ void Node::CreateEdges(const MoveList& moves) { | |
Node::ConstIterator Node::Edges() const { return {edges_, &child_}; } | ||
Node::Iterator Node::Edges() { return {edges_, &child_}; } | ||
|
||
// Recalculate n_ from real children visits. | ||
// This is needed if a node was proved to be certain in a prior | ||
// search and later gets to be root of search. | ||
void Node::RecomputeNfromChildren() { | ||
if (n_ > 1) { | ||
uint32_t visits = 1; | ||
for (const auto& child : Edges()) visits += child.GetN(); | ||
n_ = visits; | ||
} | ||
assert(n_in_flight_ == 0); | ||
} | ||
|
||
float Node::GetVisitedPolicy() const { return visited_policy_; } | ||
|
||
float Node::GetQ() const { | ||
if (parent_) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't really like relying much on a parent, as it was a temporary optimization only. Node should not go to parent to get some information about itself. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I implemented it as to be as minimal invasive to the current codebase. You are correct that overall it would be "nicer" to move from a node centric style to a edge.node or even edge centric style. I left the parts of the code as is, if they checked for terminal/certain status by node. I would propose changing this only after CP is tested and with the complete codebase and future changes in mind. So while I feel that there is currently a shift towards a more edge centric view with your new detachment scheme which might store N in the edge as well, and also even the original A0 seems rather edge centric), I would still make that a seperate PR sometime later. |
||
auto edge = parent_->GetEdgeToNode(this); | ||
if (edge->IsCertain()) return (float)edge->GetEQ(); | ||
} | ||
return q_; | ||
} | ||
|
||
Edge* Node::GetEdgeToNode(const Node* node) const { | ||
assert(node->parent_ == this); | ||
assert(node->index_ < edges_.size()); | ||
return &edges_[node->index_]; | ||
} | ||
|
||
Edge* Node::GetOwnEdge() const { return GetParent()->GetEdgeToNode(this); } | ||
Edge* Node::GetOwnEdge() const { | ||
if (GetParent()) { | ||
return GetParent()->GetEdgeToNode(this); | ||
} else | ||
return nullptr; | ||
} | ||
|
||
std::string Node::DebugString() const { | ||
std::ostringstream oss; | ||
oss << " Term:" << is_terminal_ << " This:" << this << " Parent:" << parent_ | ||
<< " Index:" << index_ << " Child:" << child_.get() | ||
<< " Sibling:" << sibling_.get() << " Q:" << q_ << " N:" << n_ | ||
<< " N_:" << n_in_flight_ << " Edges:" << edges_.size(); | ||
oss << " This:" << this << " Parent:" << parent_ << " Index:" << index_ | ||
<< " Child:" << child_.get() << " Sibling:" << sibling_.get() | ||
<< " Q:" << q_ << " N:" << n_ << " N_:" << n_in_flight_ | ||
<< " Edges:" << edges_.size(); | ||
return oss.str(); | ||
} | ||
|
||
void Node::MakeTerminal(GameResult result) { | ||
is_terminal_ = true; | ||
if (result == GameResult::DRAW) { | ||
q_ = 0.0f; | ||
} else if (result == GameResult::WHITE_WON) { | ||
q_ = 1.0f; | ||
} else if (result == GameResult::BLACK_WON) { | ||
q_ = -1.0f; | ||
} | ||
} | ||
|
||
bool Node::TryStartScoreUpdate() { | ||
if (n_ == 0 && n_in_flight_ > 0) return false; | ||
++n_in_flight_; | ||
|
@@ -358,6 +423,8 @@ void NodeTree::MakeMove(Move move) { | |
current_head_->ReleaseChildrenExceptOne(new_head); | ||
current_head_ = | ||
new_head ? new_head : current_head_->CreateSingleChildNode(move); | ||
if (current_head_->GetParent()) current_head_->GetOwnEdge()->ClearCertaintyState(); | ||
current_head_->RecomputeNfromChildren(); | ||
history_.Append(move); | ||
} | ||
|
||
|
@@ -402,8 +469,13 @@ bool NodeTree::ResetToPosition(const std::string& starting_fen, | |
// previously trimmed; we need to reset current_head_ in that case. | ||
// Also, if the current_head_ is terminal, reset that as well to allow forced | ||
// analysis of WDL hits, or possibly 3 fold or 50 move "draws", etc. | ||
if (!seen_old_head || current_head_->IsTerminal()) TrimTreeAtHead(); | ||
if (!seen_old_head) TrimTreeAtHead(); // must check if edges are created | ||
|
||
// Certainty Propagation: No need to trim the head, just resetting certainty | ||
// state and recomputing N should suffice even with WDL hits. | ||
// TODO: Works, but maybe recheck if there are crititcal edge cases. | ||
if (current_head_->GetParent()) current_head_->GetOwnEdge()->ClearCertaintyState(); | ||
current_head_->RecomputeNfromChildren(); | ||
return seen_old_head; | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment why it is needed and why 60 is picked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It slighty speeded up move generation in profiling. Number is a guestimate - 20 was to little, and 100 seemingly to much. This is not based on rigerous testing.