Open mooskagh opened 3 months ago
Here seems to be the problem :
In the file src/mcts/stoppers/stoppers.h
class PlayoutsStopper : public SearchStopper {
public:
PlayoutsStopper(int64_t limit, bool populate_remaining_playouts)
: nodes_limit_(limit ? limit : 4000000000ll), \\Here seems to be the problem
populate_remaining_playouts_(populate_remaining_playouts) {}
int64_t GetVisitsLimit() const { return nodes_limit_; }
bool ShouldStop(const IterationStats&, StoppersHints*) override;
private:
const int64_t nodes_limit_;
const bool populate_remaining_playouts_;
};
For some reason when limit = 0, nodes_limit is changed to 4000000000ll which makes go nodes 0
similar to infinite search.
A simple fix is to remove this condition. Does it affect other parts of the code ?
This 4'000'000'000 was introduced to prevent number of nodes overflowing uint32 (e.g. in go infinite
case), and we'd like to keep that check. But I believe it should be not in the stoppers constructors (so I agree with removing it from there), but in the place where the stoppers are created:
// common.cc
// "go nodes" stopper.
int64_t node_limit = 0;
if (params.nodes) {
if (options.Get<bool>(kNodesAsPlayoutsId)) {
stopper->AddStopper(std::make_unique<PlayoutsStopper>(
*params.nodes, options.Get<float>(kSmartPruningFactorId) > 0.0f));
} else {
node_limit = *params.nodes;
}
}
// always limit nodes to avoid exceeding the limit 4000000000. That number is
// default when node_limit = 0.
stopper->AddStopper(std::make_unique<VisitsStopper>(
node_limit, options.Get<float>(kSmartPruningFactorId) > 0.0f));
Also I can see that you posted a snippet from PlayoutsStopper
, but I cannot find this in the code; it's VisitsStopper
that has this check. Are you sure that you are editing the latest version?
Indeed i confused VisitsStopper
and PlayoutsStopper
, the code I was refering to is :
// src/mcts/stoppers/stoppers.h
// Watches visits (total tree nodes) and predicts remaining visits.
class VisitsStopper : public SearchStopper {
public:
VisitsStopper(int64_t limit, bool populate_remaining_playouts)
: nodes_limit_(limit ? limit : 4000000000ll),
populate_remaining_playouts_(populate_remaining_playouts) {}
int64_t GetVisitsLimit() const { return nodes_limit_; }
bool ShouldStop(const IterationStats&, StoppersHints*) override;
private:
const int64_t nodes_limit_;
const bool populate_remaining_playouts_;
};
But how exactly is this preventing uint32 overflow ? Since a check on 0 doesn't mean it has overflowed for more than 0 ? Or am I missing something
If the limit is set to 0, the nodes_limit_
should indeed be zero.
But if the limit is not set at all, it should still be 4'000'000'000
. As I said above, it's better to do in the calling code in common.cc
though, and not in the stopper itself.
It should stop immediately instead (or do one iteration when the tree is empty).