KataGo icon indicating copy to clipboard operation
KataGo copied to clipboard

Potential Division by Zero in `searchexplorehelpers.cpp`

Open ChinChangYang opened this issue 2 years ago • 2 comments
trafficstars

In the file cpp/search/searchexplorehelpers.cpp, we have the following line:

parentUtilityStdevFactor = 1.0 + searchParams.cpuctUtilityStdevScale * (parentUtilityStdev / searchParams.cpuctUtilityStdevPrior - 1.0);

During the process of searching for optimal parameters, there's a slight possibility that searchParams.cpuctUtilityStdevPrior might be zero. If this happens, we'll encounter runtime errors due to division by zero.

Suggested Fix:

To mitigate this, I recommend adding a small epsilon to the divisor to prevent it from ever being zero:

const double epsilon = 0.0000001;
parentUtilityStdevFactor = 1.0 + searchParams.cpuctUtilityStdevScale * (parentUtilityStdev / (searchParams.cpuctUtilityStdevPrior + epsilon) - 1.0);

With this epsilon addition, even if cpuctUtilityStdevPrior evaluates to a non-negative value, we can ensure the divisor is always greater than zero, which would ultimately improve the stability of our search algorithm for the optimal value of the cpuctUtilityStdevPrior parameter.

Impact:

This change would enhance the robustness of the search algorithm and reduce potential crashes or undefined behaviors due to division by zero.

ChinChangYang avatar Aug 21 '23 22:08 ChinChangYang

Instead of changing this here, it's probably better to change it here: https://github.com/lightvector/KataGo/blob/749ea21459f25aa3f1309111cadefa86b1b2ec09/cpp/program/setup.cpp#L460-L461

Simply by changing the 0.0 into an epsilon so it rejects the (slightly pathological) demand that stdev prior be 0.0.

OmnipotentEntity avatar Aug 22 '23 15:08 OmnipotentEntity

Agree.

ChinChangYang avatar Aug 22 '23 21:08 ChinChangYang