Better warnings against setting useless human sl profiles

lightvector · Jul 13, 2024 · 67a82af · 67a82af
1 parent c780e7f
commit 67a82af
Show file tree

Hide file tree

Showing 11 changed files with 30 additions and 12 deletions.
diff --git a/cpp/command/analysis.cpp b/cpp/command/analysis.cpp
@@ -197,7 +197,7 @@ int MainCmds::analysis(const vector<string>& args) {
 
   //Check for unused config keys
   cfg.warnUnusedKeys(cerr,&logger);
-  Setup::maybeWarnHumanSLParams(defaultParams,nnEval,humanEval,cerr,logger);
+  Setup::maybeWarnHumanSLParams(defaultParams,nnEval,humanEval,cerr,&logger);
 
   logger.write("Loaded config "+ cfg.getFileName());
   logger.write("Loaded model "+ modelFile);
@@ -916,6 +916,10 @@ int MainCmds::analysis(const vector<string>& args) {
             if(unusedKeys.size() > 0) {
               reportWarningForId(rbase.id, "overrideSettings", string("Unknown config params: ") + Global::concat(unusedKeys,","));
             }
+            ostringstream out;
+            if(Setup::maybeWarnHumanSLParams(rbase.params,nnEval,humanEval,out,NULL)) {
+              throw StringError(out.str());
+            }
           }
           catch(const StringError& exception) {
             reportErrorForId(rbase.id, "overrideSettings", string("Could not set settings: ") + exception.what());

diff --git a/cpp/command/evalsgf.cpp b/cpp/command/evalsgf.cpp
@@ -346,7 +346,7 @@ int MainCmds::evalsgf(const vector<string>& args) {
 
   //Check for unused config keys
   cfg.warnUnusedKeys(cerr,&logger);
-  Setup::maybeWarnHumanSLParams(params,nnEval,NULL,cerr,logger);
+  Setup::maybeWarnHumanSLParams(params,nnEval,NULL,cerr,&logger);
 
   if(rawNN) {
     NNResultBuf buf;

diff --git a/cpp/command/genbook.cpp b/cpp/command/genbook.cpp
@@ -395,7 +395,7 @@ int MainCmds::genbook(const vector<string>& args) {
 
   // Check for unused config keys
   cfg.warnUnusedKeys(cerr,&logger);
-  Setup::maybeWarnHumanSLParams(params,nnEval,NULL,cerr,logger);
+  Setup::maybeWarnHumanSLParams(params,nnEval,NULL,cerr,&logger);
 
   if(htmlDir != "")
     MakeDir::make(htmlDir);

diff --git a/cpp/command/gtp.cpp b/cpp/command/gtp.cpp
@@ -2073,7 +2073,7 @@ int MainCmds::gtp(const vector<string>& args) {
 
   //Check for unused config keys
   cfg.warnUnusedKeys(cerr,&logger);
-  Setup::maybeWarnHumanSLParams(initialGenmoveParams,engine->nnEval,engine->humanEval,cerr,logger);
+  Setup::maybeWarnHumanSLParams(initialGenmoveParams,engine->nnEval,engine->humanEval,cerr,&logger);
 
   logger.write("Loaded config " + cfg.getFileName());
   logger.write("Loaded model " + nnModelFile);
@@ -2591,6 +2591,10 @@ int MainCmds::gtp(const vector<string>& args) {
             for(const string& unused: unusedKeys) {
               throw StringError("Unrecognized or non-overridable parameter in kata-set-params: " + unused);
             }
+            ostringstream out;
+            if(Setup::maybeWarnHumanSLParams(buf1,engine->nnEval,engine->humanEval,out,NULL)) {
+              throw StringError(out.str());
+            }
           }
 
           //Ignore any unused keys in the original config so far

diff --git a/cpp/command/match.cpp b/cpp/command/match.cpp
@@ -228,7 +228,7 @@ int MainCmds::match(const vector<string>& args) {
   for(int i = 0; i<numBots; i++) {
     if(!botIsUsed[i])
       continue;
-    Setup::maybeWarnHumanSLParams(paramss[i],nnEvalsByBot[i],NULL,cerr,logger);
+    Setup::maybeWarnHumanSLParams(paramss[i],nnEvalsByBot[i],NULL,cerr,&logger);
   }
 
   //Done loading!

diff --git a/cpp/command/misc.cpp b/cpp/command/misc.cpp
@@ -427,7 +427,7 @@ int MainCmds::demoplay(const vector<string>& args) {
 
   //Check for unused config keys
   cfg.warnUnusedKeys(cerr,&logger);
-  Setup::maybeWarnHumanSLParams(params,nnEval,NULL,cerr,logger);
+  Setup::maybeWarnHumanSLParams(params,nnEval,NULL,cerr,&logger);
 
   AsyncBot* bot = new AsyncBot(params, nnEval, &logger, searchRandSeed);
   bot->setAlwaysIncludeOwnerMap(true);
@@ -1523,7 +1523,7 @@ int MainCmds::dataminesgfs(const vector<string>& args) {
 
   GameInitializer* gameInit = new GameInitializer(cfg,logger);
   cfg.warnUnusedKeys(cerr,&logger);
-  Setup::maybeWarnHumanSLParams(params,nnEval,NULL,cerr,logger);
+  Setup::maybeWarnHumanSLParams(params,nnEval,NULL,cerr,&logger);
 
   vector<string> sgfFiles;
   FileHelpers::collectSgfsFromDirsOrFiles(sgfDirs,sgfFiles);

diff --git a/cpp/program/setup.cpp b/cpp/program/setup.cpp
@@ -820,12 +820,12 @@ vector<SearchParams> Setup::loadParams(
 }
 
 
-void Setup::maybeWarnHumanSLParams(
+bool Setup::maybeWarnHumanSLParams(
   const SearchParams& params,
   const NNEvaluator* nnEval,
   const NNEvaluator* humanEval,
   std::ostream& out,
-  Logger& logger
+  Logger* logger
 ) {
   if(params.humanSLProfile.initialized) {
     bool hasAnySGFMetaUse =
@@ -840,10 +840,13 @@ void Setup::maybeWarnHumanSLParams(
           modelNames += " and ";
         modelNames += humanEval->getModelName();
       }
-      logger.write("WARNING: humanSLProfile is specified as config param but model(s) don't use it: " + modelNames);
+      if(logger != NULL)
+        logger->write("WARNING: humanSLProfile is specified as config param but model(s) don't use it: " + modelNames);
       out << "WARNING: humanSLProfile is specified as config param but model(s) don't use it: " << modelNames << endl;
+      return true;
     }
   }
+  return false;
 }
 
 

diff --git a/cpp/program/setup.h b/cpp/program/setup.h
@@ -88,12 +88,12 @@ namespace Setup {
     bool loadSingleConfigOnly
   );
 
-  void maybeWarnHumanSLParams(
+  bool maybeWarnHumanSLParams(
     const SearchParams& params,
     const NNEvaluator* nnEval,
     const NNEvaluator* humanEval,
     std::ostream& out,
-    Logger& logger
+    Logger* logger
   );
 
   Player parseReportAnalysisWinrates(

diff --git a/cpp/tests/gtp/misc.txt b/cpp/tests/gtp/misc.txt
@@ -7,3 +7,4 @@ get_komi
 
 version
 kata-get-models
+kata-set-param humanSLProfile rank_10k
diff --git a/cpp/tests/results/gtp/misc.txt.log b/cpp/tests/results/gtp/misc.txt.log
@@ -74,5 +74,8 @@ trtUseFP16 = false
 : = 1.14.1+g170-b6c96-s175M
 : Controller: kata-get-models
 : = [{"internalName":"g170-b6c96-s175395328-d26788732","maxBatchSize":8,"name":"tests/models/g170-b6c96-s175395328-d26788732.bin.gz","usesHumanSLProfile":false,"usingFP16":"false","version":8}]
+: Controller: kata-set-param humanSLProfile rank_10k
+: ? Could not set params: WARNING: humanSLProfile is specified as config param but model(s) don't use it: tests/models/g170-b6c96-s175395328-d26788732.bin.gz
+
 : GPU -1 finishing, processed 0 rows 0 batches
 : All cleaned up, quitting
diff --git a/cpp/tests/results/gtp/misc.txt.stdout b/cpp/tests/results/gtp/misc.txt.stdout
@@ -12,3 +12,6 @@
 
 = [{"internalName":"g170-b6c96-s175395328-d26788732","maxBatchSize":8,"name":"tests/models/g170-b6c96-s175395328-d26788732.bin.gz","usesHumanSLProfile":false,"usingFP16":"false","version":8}]
 
+? Could not set params: WARNING: humanSLProfile is specified as config param but model(s) don't use it: tests/models/g170-b6c96-s175395328-d26788732.bin.gz
+
+
Original file line number	Diff line number	Diff line change
Expand Up		@@ -7,3 +7,4 @@ get_komi

		version
		kata-get-models
		kata-set-param humanSLProfile rank_10k
Original file line number	Diff line number	Diff line change
Expand Up		@@ -12,3 +12,6 @@

		= [{"internalName":"g170-b6c96-s175395328-d26788732","maxBatchSize":8,"name":"tests/models/g170-b6c96-s175395328-d26788732.bin.gz","usesHumanSLProfile":false,"usingFP16":"false","version":8}]

		? Could not set params: WARNING: humanSLProfile is specified as config param but model(s) don't use it: tests/models/g170-b6c96-s175395328-d26788732.bin.gz