LeelaChessZero / lc0

The rewritten engine, originally for tensorflow. Now all other backends have been ported here.
GNU General Public License v3.0
2.37k stars 523 forks source link

fixe setoption behaviour. #1945

Closed ContradNamiseb closed 4 months ago

ContradNamiseb commented 7 months ago

This pr partially fixes issue #1843

mooskagh commented 6 months ago

It doesn't look like the code does what you described.
And I don't think that the behaviour you described is what's needed.

setoption name a name b value 1 value 2

should set an option "a name b" to value "1 value 2"

ContradNamiseb commented 6 months ago

Sorry meant to edit the comments not close the pr. I changed the described behavior of the code after talking with borg he recommended to keep this current changes in the pr.

ContradNamiseb commented 6 months ago

@mooskagh this is what I originally had.


                                 const std::string& value,
                                 const std::string& context) {
   if (name.empty()) {
      throw Exception("An option name must be provided");
    } else {
        // Split the names and values by space.
        std::vector<std::string> names = SplitString(name, ' ');
        std::vector<std::string> values = SplitString(value, ' ');
        // Check that the number of names and values match
        if (names.size() != values.size()) {
            throw Exception("Mismatched number of names and values");
        }
        // Set the UCI options for each name-value pair
        for (size_t i = 0; i < names.size(); ++i) {
            const std::string& name = names[i];
            const std::string& value = values[i];
            auto option = FindOptionByUciName(name);
            if (option) {
            option->SetValue(value, GetMutableOptions(context));
            } else {
            // If name is not valid throw an error.
            throw Exception("Unknown option: " + name);
            }
        }
        return;
    }
}```
mooskagh commented 5 months ago

It doesn't look to me that the code snippet from your latest comment does what is needed either.