Skip to content

Introduce tryGet* and getOrDefault* config parser API #1138

@KvanTTT

Description

@KvanTTT

It significantly reduces duplication, simplifies code and makes it less error prone. Also it improves parsing performance a bit (because there is only single key accessing instead of previous contains and get checks), although it's not so important.

TryGet*

Before

if(cfg.contains(backendPrefix+"DeviceToUseModel"+idxStr+"Thread"+threadIdxStr))
  gpuIdxByServerThread.push_back(cfg.getInt(backendPrefix+"DeviceToUseModel"+idxStr+"Thread"+threadIdxStr,0,1023));
else if(cfg.contains(backendPrefix+"GpuToUseModel"+idxStr+"Thread"+threadIdxStr))
  gpuIdxByServerThread.push_back(cfg.getInt(backendPrefix+"GpuToUseModel"+idxStr+"Thread"+threadIdxStr,0,1023));
else if(cfg.contains("deviceToUseModel"+idxStr+"Thread"+threadIdxStr))
  gpuIdxByServerThread.push_back(cfg.getInt("deviceToUseModel"+idxStr+"Thread"+threadIdxStr,0,1023));
else if(cfg.contains("gpuToUseModel"+idxStr+"Thread"+threadIdxStr))
  gpuIdxByServerThread.push_back(cfg.getInt("gpuToUseModel"+idxStr+"Thread"+threadIdxStr,0,1023));
else if(cfg.contains(backendPrefix+"DeviceToUseModel"+idxStr))
  gpuIdxByServerThread.push_back(cfg.getInt(backendPrefix+"DeviceToUseModel"+idxStr,0,1023));
else if(cfg.contains(backendPrefix+"GpuToUseModel"+idxStr))
  gpuIdxByServerThread.push_back(cfg.getInt(backendPrefix+"GpuToUseModel"+idxStr,0,1023));
else if(cfg.contains("deviceToUseModel"+idxStr))
  gpuIdxByServerThread.push_back(cfg.getInt("deviceToUseModel"+idxStr,0,1023));
else if(cfg.contains("gpuToUseModel"+idxStr))
  gpuIdxByServerThread.push_back(cfg.getInt("gpuToUseModel"+idxStr,0,1023));
else if(cfg.contains(backendPrefix+"DeviceToUseThread"+threadIdxStr))
  gpuIdxByServerThread.push_back(cfg.getInt(backendPrefix+"DeviceToUseThread"+threadIdxStr,0,1023));
else if(cfg.contains(backendPrefix+"GpuToUseThread"+threadIdxStr))
  gpuIdxByServerThread.push_back(cfg.getInt(backendPrefix+"GpuToUseThread"+threadIdxStr,0,1023));
else if(cfg.contains("deviceToUseThread"+threadIdxStr))
  gpuIdxByServerThread.push_back(cfg.getInt("deviceToUseThread"+threadIdxStr,0,1023));
else if(cfg.contains("gpuToUseThread"+threadIdxStr))
  gpuIdxByServerThread.push_back(cfg.getInt("gpuToUseThread"+threadIdxStr,0,1023));
else if(cfg.contains(backendPrefix+"DeviceToUse"))
  gpuIdxByServerThread.push_back(cfg.getInt(backendPrefix+"DeviceToUse",0,1023));
else if(cfg.contains(backendPrefix+"GpuToUse"))
  gpuIdxByServerThread.push_back(cfg.getInt(backendPrefix+"GpuToUse",0,1023));
else if(cfg.contains("deviceToUse"))
  gpuIdxByServerThread.push_back(cfg.getInt("deviceToUse",0,1023));
else if(cfg.contains("gpuToUse"))
  gpuIdxByServerThread.push_back(cfg.getInt("gpuToUse",0,1023));
else
  gpuIdxByServerThread.push_back(-1);

After

int idx = -1;
constexpr int min = 0;
constexpr int max = 1023;
(void)(cfg.tryGetInt(backendPrefix+"DeviceToUseModel"+idxStr+"Thread"+threadIdxStr, idx, min, max) ||
  cfg.tryGetInt(backendPrefix+"GpuToUseModel"+idxStr+"Thread"+threadIdxStr, idx, min, max) ||
  cfg.tryGetInt("deviceToUseModel"+idxStr+"Thread"+threadIdxStr, idx, min, max) ||
  cfg.tryGetInt("gpuToUseModel"+idxStr+"Thread"+threadIdxStr, idx, min, max) ||
  cfg.tryGetInt(backendPrefix+"DeviceToUseModel"+idxStr, idx, min, max) ||
  cfg.tryGetInt(backendPrefix+"GpuToUseModel"+idxStr, idx, min, max) ||
  cfg.tryGetInt("deviceToUseModel"+idxStr, idx, min, max) ||
  cfg.tryGetInt("gpuToUseModel"+idxStr, idx, min, max) ||
  cfg.tryGetInt(backendPrefix+"DeviceToUseThread"+threadIdxStr, idx, min, max) ||
  cfg.tryGetInt(backendPrefix+"GpuToUseThread"+threadIdxStr, idx, min, max) ||
  cfg.tryGetInt("deviceToUseThread"+threadIdxStr, idx, min, max) ||
  cfg.tryGetInt("gpuToUseThread"+threadIdxStr, idx, min, max) ||
  cfg.tryGetInt(backendPrefix+"DeviceToUse", idx, min, max) ||
  cfg.tryGetInt(backendPrefix+"GpuToUse", idx, min, max) ||
  cfg.tryGetInt("deviceToUse", idx, min, max) ||
  cfg.tryGetInt("gpuToUse", idx, min, max));
gpuIdxByServerThread.push_back(idx);

get*OrDefault

Before

komiMean = cfg.contains("komiMean") ? cfg.getFloat("komiMean",Rules::MIN_USER_KOMI,Rules::MAX_USER_KOMI) : 7.5f;
komiStdev = cfg.contains("komiStdev") ? cfg.getFloat("komiStdev",0.0f,60.0f) : 0.0f;
handicapProb = cfg.contains("handicapProb") ? cfg.getDouble("handicapProb",0.0,1.0) : 0.0;
handicapCompensateKomiProb = cfg.contains("handicapCompensateKomiProb") ? cfg.getDouble("handicapCompensateKomiProb",0.0,1.0) : 0.0;
komiBigStdevProb = cfg.contains("komiBigStdevProb") ? cfg.getDouble("komiBigStdevProb",0.0,1.0) : 0.0;
komiBigStdev = cfg.contains("komiBigStdev") ? cfg.getFloat("komiBigStdev",0.0f,60.0f) : 10.0f;
komiBiggerStdevProb = cfg.contains("komiBiggerStdevProb") ? cfg.getDouble("komiBiggerStdevProb",0.0,1.0) : 0.0;
komiBiggerStdev = cfg.contains("komiBiggerStdev") ? cfg.getFloat("komiBiggerStdev",0.0f,120.0f) : 30.0f;
handicapKomiInterpZeroProb = cfg.contains("handicapKomiInterpZeroProb") ? cfg.getDouble("handicapKomiInterpZeroProb",0.0,1.0) : 0.0;
sgfKomiInterpZeroProb = cfg.contains("sgfKomiInterpZeroProb") ? cfg.getDouble("sgfKomiInterpZeroProb",0.0,1.0) : 0.0;
komiAuto = cfg.contains("komiAuto") ? cfg.getBool("komiAuto") : false;
forkCompensateKomiProb = cfg.contains("forkCompensateKomiProb") ? cfg.getDouble("forkCompensateKomiProb",0.0,1.0) : handicapCompensateKomiProb;
sgfCompensateKomiProb = cfg.contains("sgfCompensateKomiProb") ? cfg.getDouble("sgfCompensateKomiProb",0.0,1.0) : forkCompensateKomiProb;
komiAllowIntegerProb = cfg.contains("komiAllowIntegerProb") ? cfg.getDouble("komiAllowIntegerProb",0.0,1.0) : 1.0;

After

komiMean = cfg.getFloatOrDefault("komiMean", Rules::MIN_USER_KOMI, Rules::MAX_USER_KOMI, 7.5f);
komiStdev = cfg.getFloatOrDefault("komiStdev",0.0f, 60.0f, 0.0f);
handicapProb = cfg.getDoubleOrDefault("handicapProb", 0.0, 1.0, 0.0);
handicapCompensateKomiProb = cfg.getDoubleOrDefault("handicapCompensateKomiProb", 0.0, 1.0, 0.0);
komiBigStdevProb = cfg.getDoubleOrDefault("komiBigStdevProb",0.0,1.0, 0.0);
komiBigStdev = cfg.getFloatOrDefault("komiBigStdev", 0.0f, 60.0f, 10.0f);
komiBiggerStdevProb = cfg.getDoubleOrDefault("komiBiggerStdevProb", 0.0, 1.0, 0.0);
komiBiggerStdev = cfg.getFloatOrDefault("komiBiggerStdev", 0.0f, 120.0f, 30.0f);
handicapKomiInterpZeroProb = cfg.getDoubleOrDefault("handicapKomiInterpZeroProb", 0.0, 1.0, 0.0);
sgfKomiInterpZeroProb = cfg.getDoubleOrDefault("sgfKomiInterpZeroProb", 0.0, 1.0, 0.0);
komiAuto = cfg.getBoolOrDefault("komiAuto", false);
forkCompensateKomiProb = cfg.getDoubleOrDefault("forkCompensateKomiProb", 0.0, 1.0, handicapCompensateKomiProb);
sgfCompensateKomiProb = cfg.getDoubleOrDefault("sgfCompensateKomiProb", 0.0, 1.0, forkCompensateKomiProb);
komiAllowIntegerProb = cfg.getDoubleOrDefault("komiAllowIntegerProb", 0.0, 1.0, 1.0);

Misc

Before

if(cfg.contains("maxPlayouts"+idxStr)) params.maxPlayouts = cfg.getInt64("maxPlayouts"+idxStr, (int64_t)1, (int64_t)1 << 50);
else if(cfg.contains("maxPlayouts"))   params.maxPlayouts = cfg.getInt64("maxPlayouts",        (int64_t)1, (int64_t)1 << 50);

After

params.maxPlayouts = getValueForBot<int64_t>(cfg, "maxPlayouts", idxStr, 1, static_cast<int64_t>(1) << 50, params.maxPlayouts);

Implementation details

I generalized code of getters by introducing some helper methods like the following:

template<typename T>
T ConfigParser::parseOrError(const string& key, const string& str, const T min, const T max) {
  T x;
  bool success = false;
  if constexpr (std::is_same_v<T, int>) success = Global::tryStringToInt(str, x);
  else if constexpr (std::is_same_v<T, int64_t>) success = Global::tryStringToInt64(str, x);
  else if constexpr (std::is_same_v<T, uint64_t>) success = Global::tryStringToUInt64(str, x);
  else if constexpr (std::is_same_v<T, float>) success = Global::tryStringToFloat(str, x);
  else if constexpr (std::is_same_v<T, double>) success = Global::tryStringToDouble(str, x);
  else if constexpr (std::is_same_v<T, bool>) success = Global::tryStringToBool(str, x);
  else if constexpr (std::is_same_v<T, enabled_t>) success = enabled_t::tryParse(Global::toLower(str), x);

  if(!success)
    throw IOError("Could not parse '" + str + "' for key '" + key + "' in config file " + fileName);

  if constexpr (std::is_arithmetic_v<T>) {
    assert(min <= max);

    if constexpr (std::is_floating_point_v<T>) {
      if(std::isnan(x))
        throw IOError("Key '" + key + "' is nan in config file " + fileName);
    }

    if (x < min || x > max) {
      stringstream ss;
      ss << "Key '" << key << "' must be in the range " << min << " to " << max << " in config file " << fileName;
      throw IOError(ss.str());
    }
  }

  return x;
}

I've already implemented the API in my fork and I can backport the changes into upstream (if it will be reviewed and merged): KvanTTT#21

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions