-
Notifications
You must be signed in to change notification settings - Fork 571
Additional reporting details for selfplay to support T50+ #821
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
Conversation
@@ -117,15 +119,18 @@ void SelfPlayGame::Play(int white_threads, int black_threads, bool training, | |||
eval = (eval + 1) / 2; | |||
if (eval < min_eval_[idx]) min_eval_[idx] = eval; | |||
const int move_number = tree_[0]->GetPositionHistory().GetLength() / 2 + 1; | |||
auto best_w = (best_eval.first + 1.0f - best_eval.second) / 2.0f; |
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.
Maybe add struct WDL { float win; float draw; float loss; }
and use it instead? (for best_*
and also for max_eval_
).
I think even without constructor, curly braces initialization will work:
WDL best{(best_eval.first + 1.0f - best_eval.second) / 2.0f,
best_eval.second,
best_w - best_eval.first};
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.
Feels a bit like overkill.
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.
For me it doesn't, but ok. :)
@@ -171,6 +176,15 @@ std::vector<Move> SelfPlayGame::GetMoves() const { | |||
} | |||
|
|||
float SelfPlayGame::GetWorstEvalForWinnerOrDraw() const { | |||
if (options_[0].uci_options->Get<bool>(kResignWDLStyleId.GetId())) { |
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.
Is it theoretically possible that player1 and player2 have different kResignWDLStyle values set? Then options_[0]
won't work.
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.
Theoretically - yes. But not using the same resign style for both sides seems pretty silly.
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.
If it's <=3 lines of code change to add it, would be nice to support it to avoid future surprises.
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.
Actually its not. Its not the final position that matters for this function, its every single position so far. So its a bunch more work to support them having different values.
* Hack to report average nodes per move for monitoring training. * WDLstyle resign false positive reporting * Make this less WIP and more submittable. * Cast correctly. * Fix typo in last commit... * Add comment about unsupported combination.
…Zero#821) * Hack to report average nodes per move for monitoring training. * WDLstyle resign false positive reporting * Make this less WIP and more submittable. * Cast correctly. * Fix typo in last commit... * Add comment about unsupported combination.
No description provided.