Skip to content
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

backend to "switch" to a different net for the endgame #1968

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

borg323
Copy link
Member

@borg323 borg323 commented Mar 6, 2024

The net is switched when the number of pieces on board, excluding pawns and kings, reaches a threshold, 6 by default. The net to switch to is selected using the endgame_weights backend option. Also two subdictionaries are accepted in the backend options, namely main and endgame to allow using different backend options with the respective net. Unsurprisingly, the threshold is set using the threshold backend option.

@@ -124,6 +124,13 @@ class Lexer {
static const std::string kNumberChars = "0123456789-.";
if (kNumberChars.find(str_[idx_]) != std::string::npos) {
ReadNumber();
// If the next character isn't a separator, this is probably an identifier
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe IsAlpha() would work better? Otherwise we'd need to remember to check for more characters here if we extend the syntax.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem I'm trying to address here is that strings starting with a number (e.g. a typical net name like 744204.pb.gz) are identified as a number and parsing fails at the first unexpected character - it is easier to blacklist a couple of characters than to whitelist dozens.


class SwitchComputation : public NetworkComputation {
public:
SwitchComputation(std::unique_ptr<NetworkComputation> main_comp,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(totally optional)
I'd do it more generic, with a vector of computation+thresholds..


void ComputeBlocking() override {
if (threads_ > 1 && main_cnt_ > 0 && endgame_cnt_ > 0) {
std::thread main(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that starting a thread is usually slow, especially on linux.
In other backends, e.g. network_mux.cc we did condvar-based synchronization instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to test the waters. If it shows potential we can consider a better performing threading approach. (Note the default it one thread and I didn't mention it in the description).

int GetBatchSize() const override { return main_cnt_ + endgame_cnt_; }

float GetQVal(int sample) const override {
if (is_endgame_[sample]) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe helper function

NetworkComputation* GetComp(int sample) { return is_endgame_[sample] ? endgame_comp : main_comp_; }

?

const OptionsDict& options) {
auto backends = NetworkFactory::Get()->GetBackendsList();

threshold_ = options.GetOrDefault<int>("threshold", 6);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without looking into the code, neither variable nor the parameter name's meaning "threshold" was clear. Maybe something with pieces in the name? pieces_threshold? piece_count? :-\ Not sure what would be better though.

}

bool IsCpu() const override {
return main_net_->GetMiniBatchSize() | endgame_net_->GetMiniBatchSize();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsCpu()

src/neural/network_blend.cc Outdated Show resolved Hide resolved
src/neural/network_blend.cc Outdated Show resolved Hide resolved
src/neural/network_blend.cc Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants