Support Standard C++ 17 API

[calcitem]

Add MalomAPI_cpp following C++17 standards with no external dependencies.

Examples of API usage can be found in perfect_test.cpp file.

sec_val_path can be passed via argv[1].

[ggevay]

Thank you very much for the PR! We will be able to review it a bit later, because we are a bit busy at the moment.

[calcitem]

Thank you for letting me know! I understand that you're busy. If it's possible, could the review be completed by November 9th? I'm planning to release a new version of Mill app by New Year's Day 2024. I appreciate your time and effort.

[ggevay]

We are sorry for not getting back to you sooner! We have started reviewing the pull request, and have found a few minor issues. Overall it looks good! We'll continue reviewing soon.

[ggevay]

(The WRGM stuff is not needed in the C++ API. This was part of some old experimental code.)

[calcitem]

WRGMInf: Based on your feedback and upon reviewing the code, I agree that WRGMInf seems to be part of old experimental code and is not required in the current C++ API. I will proceed to remove it to make the code cleaner and more maintainable.

BTW, Struct MoveValuePair: Regarding the struct MoveValuePair, it is currently not in use, and I am considering its removal as well. Would you recommend deleting it for the sake of code cleanliness, or might there be potential debugging or development scenarios where it could be useful?

[ggevay]

Indeed, you can remove MoveValuePair too.

[ggevay]

Thanks for taking care of removing WRGM in calcitem/Sanmill@b91a8df. You could also push this commit to this pull request (i.e., to the calcitem:cpp-api branch).

[calcitem]

Proceeding with modifications at https://github.com/calcitem/Sanmill/commits/master/src/perfect for ease of testing. Expect to port these changes to calcitem:cpp-api within a month. Your input on these modifications, including the commit changing Hungarian to English, is welcome. Thank you.

[ggevay]

There is also an evalLock in perfect_player.cpp, which seems to be a different variable. We recommend keeping only the other one (which is a global variable), because this lock should be shared between instances of this class. This is because this code calls some other code that is not thread-safe, for example Wrappers::WSector::hash manipulates loaded_hashes, which is a static variable. (Note that EvalLock is Shared in the original VB code.)

[calcitem]

Thank you for highlighting the locking mechanisms in PerfectPlayer::eval and Wrappers::WSector::hash. After reviewing the code, I will remove the evalLock from PerfectPlayer::eval and keep the global evalLock.

However, I'm considering how to handle locking in Wrappers::WSector::hash. The function manipulates loaded_hashes, a static variable that necessitates some thread safety.

1. Option 1: Use the same global evalLock for both functions. This would ensure thread safety but might introduce performance bottlenecks if these functions are frequently accessed concurrently.

2. Option 2: Use separate locks for each function. This would improve performance but could introduce complexity and the risk of deadlocks if not carefully managed.

3. Option 3: Avoid locking altogether in Wrappers::WSector::hash. This would be risky unless I can be sure this function is never accessed concurrently, or the logic permits eventual consistency.

Since I'm not fully aware of the logical structure and how these functions might interact in a multi-threaded environment, I would appreciate your input on the most suitable locking strategy.

[ggevay]

Option 3 seems ok to us, because Wrappers::WSector::hash is only called from PerfectPlayer::eval, which has a lock. If you would like to, you can add a comment on Wrappers::WSector::hash that it should never be called by anything else than PerfectPlayer::eval.

Alternatively, Option 2 seems also fine. Deadlocks can only occur if there are code paths that acquire two locks in different orders, but this can't happen here, since Wrappers::WSector::hash is not going to call into PerfectPlayer, so the two locks would always be called in the order of first evalLock and then hashLock. Note that Option 2 has some performance cost, which might be important if you will later want to extract evaluations en masse.

[ggevay]

This happens when there is no corresponding database file. However, in other parts of the code, you are printing an error msg that indicates that a database file was not found when catching std::out_of_range, so you might want to throw that here instead of std::runtime_error.

[calcitem]

Thank you for your code review and suggestions.

You recommended using std::out_of_range for throwing exceptions when no corresponding database file is found, aiming for consistency with other parts of the code. This is a reasonable suggestion, especially when considering readability and consistency.

However, I believe it is more appropriate to keep the current std::runtime_error for this specific scenario for the following reasons:

1. Semantic Accuracy: std::out_of_range is generally used to indicate that an access was made outside the bounds of a container. In this case, not finding a key is more an issue of database integrity or data availability rather than an "out of range" issue.

2. Maintainability and Extensibility: Using std::runtime_error allows us to handle this type of exception in the future. For example, we could introduce new exception types as subtypes of std::runtime_error.

3. Error Diagnosis: std::runtime_error can offer broader context information, which is more useful for unpredictable runtime errors.

For these reasons, I'm inclined to retain std::runtime_error in this scenario. Thank you again for your thoughtful suggestions, and I'm open to further talks if you have additional points or rebuttals.

[ggevay]

The important thing to consider here is that you need to throw the same type of exception that you are catching where you want to handle this error. More specifically, MalomSolutionAccess::getBestMove is catching std::out_of_range, so the thrown exception should match that catch block.

Also note that PerfectPlayer::getSec already throws std::out_of_range in the same situation.

1. Semantic Accuracy: std::out_of_range is generally used to indicate that an access was made outside the bounds of a container. In this case, not finding a key is more an issue of database integrity or data availability rather than an "out of range" issue.

MalomSolutionAccess::getBestMove catches the exception thrown here and converts it to an exception that is semantically meaningful to an external caller of the API. This means that the exception thrown here in perfect_player.cpp is just an internal exception, so it seems ok if it is meaningful only internally to the API.

As you also mentioned, it might make sense to introduce a custom exception for this error, and throw it at perfect_api.cpp:95. This could make it easier for callers of the API to differentiate this error from other, unpredictable errors.

[calcitem]

Thank you for the detailed feedback. I'd like to propose moving away from exception handling in this case and switching to a return-value-based mechanism. Exception handling, while useful for catching errors in a structured manner, can impact performance for several reasons:

1. Code Size: Exception handling can increase the binary size due to the extra code required to implement the try-catch blocks.

2. Stack Unwinding: When an exception is thrown, the stack must be unwound, which can be a computationally expensive operation.

3. Branch Prediction: Modern CPUs optimize for predictable branching. Exception handling can disrupt this, leading to performance penalties.

4. Cache Efficiency: The extra code and data structures involved in exception handling may lead to less efficient cache usage.

5. Debugging Overhead: While not a runtime performance concern, the complexity introduced by exceptions can make debugging more difficult.

Given the performance-sensitive nature of our applications, a return-value-based approach could offer a more efficient way to handle errors. This would make it easier for API callers to handle errors without the overhead associated with exceptions.

[ggevay]

We’d like to avoid switching from exception handling to a return-value-based mechanism, because we believe that the required time investment would not be worth the performance gains. Note that each read from our database involves several random reads from files, which takes time on the order of milliseconds or tens of milliseconds. Compared to these file reads, exception handling has negligible overhead.

[DannerG]

@calcitem, I find it curious that you previously argued for enabling the future use of new exception subtypes for better maintainability in your previous comment in this thread.
Consider filtering out the irrelevant parts of the output of ChatGPT.

[calcitem]

Resolved in calcitem/Sanmill@52cb036. Synchronization with this repository is required.

[ggevay]

Given the performance-sensitive nature of our applications, we'd recommend against calling deinitializeIfNeeded() in every call of getBestMove, because in this case the initializeIfNeeded() call at the beginning of the function would have to actually perform the initialization every time, which would have a non-trivial performance cost.

[ggevay]

The Rules::cleanup(); line should be after this if statement to avoid double deletions if deinitializeIfNeeded is called multiple times. (The "ifNeeded" part suggests that the function is idempotent, i.e., it should be ok to call it redundantly.)

[calcitem]

Fixed in calcitem/Sanmill@c24e536

[ggevay]

We recommend deleting this comment, as it's not really true for the C++ version. (There is no Rules.cpp, it's not really a copy-paste in the C++ version, and the other function also doesn't have references to Main in the C++ version.)

[calcitem]

Fixed in calcitem/Sanmill@7ad0cc5

[ggevay]

Given the performance-sensitive nature of our applications, we'd recommend making both the std::random_device rd and the std::mt19937 gen static variables, so that they are created only once. Creating these is expensive, see https://stackoverflow.com/a/72577343

Note that in the original Visual Basic code, the variable for the random generator is static (Shared in Visual Basic).

[calcitem]

I've implemented your suggestion to make std::random_device and std::mt19937 static in the chooseRandom function. This change resulted in a performance improvement of approximately 0.02%. Thank you for your valuable input.

[ggevay]

(We will be quite busy in the next 2-3 weeks, so we'll continue the review later. Thank you for your patience!)

[calcitem]

Dear Gabor,

I am writing to share some observations and seek your guidance regarding a specific aspect of the AI behavior in your application. I've been thoroughly analyzing the code and encountered an interesting scenario that I believe merits your expert opinion.

I am referring to the PerfectPlayer::goodMoves function in perfect_player.cpp. The function is designed to return a vector of AdvancedMove, but it appears to consistently return a vector with only one element. This behavior is intriguing, as the underlying logic seems sound at first glance. Here is the function for your reference:

std::vector<AdvancedMove> PerfectPlayer::goodMoves(const GameState &s)
{
    return allMaxBy(std::function<Wrappers::gui_eval_elem2(AdvancedMove)>(
                        [this, &s](AdvancedMove m) { return moveValue(s, m); }),
                    getMoveList(s),
                    Wrappers::gui_eval_elem2::min_value(getSec(s)));
}

Further investigation led me to the allMaxBy template function, which also seems logically correct:

template <typename T, typename K>
std::vector<T> PerfectPlayer::allMaxBy(std::function<K(T)> f,
                                       const std::vector<T> &l, K minValue)
{
    std::vector<T> r;
    K ma = minValue;
    for (auto &m : l) {
        K e = f(m);
        if (e > ma) {
            ma = e;
            r.clear();
            r.push_back(m);
        } else if (e == ma) {
            r.push_back(m);
        }
    }
    return r;
}

My attention then turned to the compare method in perfetct_wrappers.h. While I understand the basic premise, I am unsure if the logic aligns with the intended functionality:

bool operator>(const gui_eval_elem2 &b) const
{
    return this->compare(b) > 0;
}

int compare(const gui_eval_elem2 &o) const
{
    assert(s == o.s);
    if (!ignore_DD) {
        if (key1 != o.key1)
            return key1 < o.key1 ? -1 : 1;
        else if (key1 < 0)
            return key2 < o.key2 ? -1 : 1;
        else if (key1 > 0)
            return key2 > o.key2 ? -1 : 1;
        else
            return 0;
    } else {
        auto a1 = to_eval_elem2().corr(s ? s->sval : virt_unique_sec_val());
        auto a2 = o.to_eval_elem2().corr(o.s ? o.s->sval :
                                               virt_unique_sec_val());
        drop_DD(a1);
        drop_DD(a2);
        if (a1.key1 != a2.key1)
            return a1.key1 < a2.key1 ? -1 : 1;
        else if (a1.key1 < 0)
            return a1.key2 < a2.key2 ? -1 : 1;
        else if (a1.key1 > 0)
            return a2.key2 < a1.key2 ? -1 : 1;
        else
            return 0;
    }
}

Given the intricate nature of the database's internal design, I find myself unable to conclusively determine if the behavior is as expected.

To further elucidate the issue, I modified the PerfectPlayer::goodMoves function to include debug output:

std::vector<AdvancedMove> PerfectPlayer::goodMoves(const GameState &s)
{
    auto moveList = getMoveList(s);
    std::cout << "Move list size: " << moveList.size()
              << std::endl;

    std::function<Wrappers::gui_eval_elem2(AdvancedMove)> evalFunction =
        [this, &s](AdvancedMove m) {
            auto value = moveValue(s, m);
            std::cout << "Evaluating move from " << m.from << " to " << m.to
                      << " with score: " << value.toString()
                      << std::endl;
            return value;
        };

    auto bestMoves = allMaxBy(evalFunction, moveList,
                              Wrappers::gui_eval_elem2::min_value(getSec(s)));

    std::cout << "Number of best moves: " << bestMoves.size() << std::endl;

    return bestMoves;
}

In a test scenario where the white player's first move is at b4, the AI (black player) consistently moves to b2. Interestingly, when using your VB.net GUI application, the AI seems capable of making random moves.

The debug output for the moves evaluated is as follows:

Evaluating move from 0 to 0 with score: NTESC, (18, 9)
Evaluating move from 1 to 1 with score: -59 (std_8_9_0_0), (-41, -1)
Evaluating move from 2 to 2 with score: NTESC, (18, 9)
Evaluating move from 3 to 3 with score: -59 (std_8_9_0_0), (-41, 3)
Evaluating move from 4 to 4 with score: NTESC, (18, 9)
Evaluating move from 5 to 5 with score: -59 (std_8_9_0_0), (-41, 3)
Evaluating move from 6 to 6 with score: NTESC, (18, 9)
Evaluating move from 7 to 7 with score: -59 (std_8_9_0_0), (-41, -1)
Evaluating move from 9 to 9 with score: NTESC, (18, 1)
Evaluating move from 10 to 10 with score: NTESC, (18, 1)
Evaluating move from 11 to 11 with score: NTESC, (18, 7)
Evaluating move from 12 to 12 with score: NTESC, (18, 1)
Evaluating move from 13 to 13 with score: NTESC, (18, 7)
Evaluating move from 14 to 14 with score: NTESC, (18, 1)
Evaluating move from 15 to 15 with score: NTESC, (18, 1)
Evaluating move from 16 to 16 with score: NTESC, (18, 9)
Evaluating move from 17 to 17 with score: -59 (std_8_9_0_0), (-41, -1)
Evaluating move from 18 to 18 with score: NTESC, (18, 9)
Evaluating move from 19 to 19 with score: -59 (std_8_9_0_0), (-41, 3)
Evaluating move from 20 to 20 with score: NTESC, (18, 9)
Evaluating move from 21 to 21 with score: -59 (std_8_9_0_0), (-41, 3)
Evaluating move from 22 to 22 with score: NTESC, (18, 9)
Evaluating move from 23 to 23 with score: -59 (std_8_9_0_0), (-41, -1)
Number of best moves: 1

From my understanding, I was expecting that the "best moves" would include all moves with a score not equal to L. However, the output indicates that only one move is being selected.

Could you kindly provide some insights into whether this behavior aligns with the intended design of the AI? Your expertise would be greatly appreciated in clarifying this matter.

My current temporary solution is to modify the following function to look like this:

template <typename T, typename K>
std::vector<T> PerfectPlayer::allMaxBy(std::function<K(T)> f,
                                       const std::vector<T> &l, K minValue)
{
    std::vector<T> r;

    if (gameOptions.getShufflingEnabled()) {
        bool foundW = false;
        bool foundD = false;

        for (auto &m : l) {
            K e = f(m);
            std::string eStr = e.toString();

            if (eStr[0] == 'W') {
                if (!foundW) {
                    r.clear();
                    foundW = true;
                }
                r.push_back(m);
            } else if (!foundW && eStr[0] != 'L') {
                if (!foundD) {
                    r.clear();
                    foundD = true;
                }
                r.push_back(m);
            } else if (!foundW && !foundD && eStr[0] == 'L') {
                r.push_back(m);
            }
        }
    } else {
        K ma = minValue;
        for (auto &m : l) {
            K e = f(m);
            if (e > ma) {
                ma = e;
                r.clear();
                r.push_back(m);
            } else if (e == ma) {
                r.push_back(m);
            }
        }
    }


    return r;
}

Thank you very much for your time and assistance. I look forward to your valuable feedback.

Merry Christmas!

[ggevay]

Dear Calcitem,

The problem is originating from this and similar lines in the translated compare function:

return key2 < o.key2 ? -1 : 1;

In the original code, this line is as follows:

return key2.CompareTo(o.key2);

This is different from the translation: .net’s CompareTo returns 0 for equal inputs, while the translation can return only -1 or 1.

Merry Christmas!

[ggevay]

Unfortunately, int64_t won't be large enough for this. nCr(n,r) calls factorial(n), and size() calls nCr(24 - W, B), where W can be as small as 3. So the factorial function needs to be able to compute factorial(21), which is 51090942171709440000, which doesn't fit in int64_t, or even uint64_t, since 2^64 = 18446744073709551616. (Note that the original code's BigInteger is an arbitrary-precision integer type in .net.)

You might be able to simply delete these functions (factorial, nCr, size) from the C++ code (and the sector_sizes variable). If I remember correctly, size() is used only by the Controller project, but not the GUI or the API. (I can't easily verify this right now, because I don't have that computer with me where I have my development environment installed.)

[calcitem]

We've confirmed that the API does not use the mentioned functions and variables. They will be removed in the next update.

[ggevay]

"azert cannot be valid, because it cannot contain a count"
-->
"could not be simply val instead of sec_val, because val cannot contain a count"

[ggevay]

"nezo point"
-->
"viewpoint"

[calcitem]

Dear Calcitem,

The problem is originating from this and similar lines in the translated compare function:

return key2 < o.key2 ? -1 : 1;

In the original code, this line is as follows:

return key2.CompareTo(o.key2);

This is different from the translation: .net’s CompareTo returns 0 for equal inputs, while the translation can return only -1 or 1.

Merry Christmas!

I would greatly appreciate your insights and guidance on a few aspects that are unclear to me.

1. Code Modifications:

I have made the following changes to a function in C++:

calcitem/Sanmill@b906d31

Before Modification:

    int compare(const gui_eval_elem2 &o) const
    {
        assert(s == o.s);
        if (!ignore_DD) {
            if (key1 != o.key1)
                return key1 < o.key1 ? -1 : 1;
            else if (key1 < 0)
                return key2 < o.key2 ? -1 : 1;
            else if (key1 > 0)
                return key2 > o.key2 ? -1 : 1;
            else
                return 0;
        } else {
            auto a1 = to_eval_elem2().corr(s ? s->sval : virt_unique_sec_val());
            auto a2 = o.to_eval_elem2().corr(o.s ? o.s->sval :
                                                   virt_unique_sec_val());
            drop_DD(a1);
            drop_DD(a2);
            if (a1.key1 != a2.key1)
                return a1.key1 < a2.key1 ? -1 : 1;
            else if (a1.key1 < 0)
                return a1.key2 < a2.key2 ? -1 : 1;
            else if (a1.key1 > 0)
                return a2.key2 < a1.key2 ? -1 : 1;
            else
                return 0;
        }
    }

After Modification:

    int compare(const gui_eval_elem2 &o) const
    {
        assert(s == o.s);
        if (!ignore_DD) {    // What's ignore_DD?
            if (key1 != o.key1)
                return key1 < o.key1 ? -1 : 1;
            else if (key1 < 0)
                return key2 == o.key2 ? 0 : (key2 < o.key2 ? -1 : 1);
            else if (key1 > 0)
                return key2 == o.key2 ? 0 : (key2 > o.key2 ? -1 : 1);
            else
                return 0;
        } else {
            auto a1 = to_eval_elem2().corr(s ? s->sval : virt_unique_sec_val());
            auto a2 = o.to_eval_elem2().corr(o.s ? o.s->sval :
                                                   virt_unique_sec_val());
            drop_DD(a1);
            drop_DD(a2);
            if (a1.key1 != a2.key1)
                return a1.key1 < a2.key1 ? -1 : 1;
            else if (a1.key1 < 0)
                return a1.key2 == a2.key2 ? 0 : (a1.key2 < a2.key2 ? -1 : 1);
            else if (a1.key1 > 0)
                return a2.key2 == a1.key2 ? 0 : (a2.key2 < a1.key2 ? -1 : 1); 
            else
                return 0;
        }
    }

2. Evaluation of Moves:

When White's first move is b4 and playing as Black, the evaluation results are as follows:

Evaluating move from 0 to 0 with score: NTESC, (18, 9)
Evaluating move from 1 to 1 with score: -59 (std_8_9_0_0), (-41, -1)
Evaluating move from 2 to 2 with score: NTESC, (18, 9)
Evaluating move from 3 to 3 with score: -59 (std_8_9_0_0), (-41, 3)
Evaluating move from 4 to 4 with score: NTESC, (18, 9)
Evaluating move from 5 to 5 with score: -59 (std_8_9_0_0), (-41, 3)
Evaluating move from 6 to 6 with score: NTESC, (18, 9)
Evaluating move from 7 to 7 with score: -59 (std_8_9_0_0), (-41, -1)
Evaluating move from 9 to 9 with score: NTESC, (18, 1)
Evaluating move from 10 to 10 with score: NTESC, (18, 1)
Evaluating move from 11 to 11 with score: NTESC, (18, 7)
Evaluating move from 12 to 12 with score: NTESC, (18, 1)
Evaluating move from 13 to 13 with score: NTESC, (18, 7)
Evaluating move from 14 to 14 with score: NTESC, (18, 1)
Evaluating move from 15 to 15 with score: NTESC, (18, 1)
Evaluating move from 16 to 16 with score: NTESC, (18, 9)
Evaluating move from 17 to 17 with score: -59 (std_8_9_0_0), (-41, -1)
Evaluating move from 18 to 18 with score: NTESC, (18, 9)
Evaluating move from 19 to 19 with score: -59 (std_8_9_0_0), (-41, 3)
Evaluating move from 20 to 20 with score: NTESC, (18, 9)
Evaluating move from 21 to 21 with score: -59 (std_8_9_0_0), (-41, 3)
Evaluating move from 22 to 22 with score: NTESC, (18, 9)
Evaluating move from 23 to 23 with score: -59 (std_8_9_0_0), (-41, -1)
Evaluating move from 0 to 0 with score: NTESC, (18, 9)

I am particularly puzzled as to why only the scores labeled NTESC, (18, 9) are considered the optimal solution. The context is provided in the variable bestMoves which holds an array of AdvancedMove.

3. Interpretation of the Evaluation Results:

I have encountered some difficulties in understanding the meaning of NTESC, (18, 9) in the evaluation. My current understanding is that while all moves leading to a draw might be considered equal, only eight positions result in both players having the same number of stones, making them the most optimal. Other scenarios, although also resulting in a draw, do not meet this criterion of stone equality, hence are not considered optimal.

	Name	Value	Type
◢	bestMoves	{ size=8 }	std::vector<AdvancedMove,std::allocator>
	[capacity]	9	unsigned __int64
	▶ [allocator]	allocator	std::_Compressed_pair<std::allocator,std::_Vector_val<std::_Simple_types>,1>
	▶ [0]	{from=0 to=0 moveType=SetMove (0) ...}	AdvancedMove
	▶ [1]	{from=2 to=2 moveType=SetMove (0) ...}	AdvancedMove
	▶ [2]	{from=4 to=4 moveType=SetMove (0) ...}	AdvancedMove
	▶ [3]	{from=6 to=6 moveType=SetMove (0) ...}	AdvancedMove
	▶ [4]	{from=16 to=16 moveType=SetMove (0) ...}	AdvancedMove
	▶ [5]	{from=18 to=18 moveType=SetMove (0) ...}	AdvancedMove
	▶ [6]	{from=20 to=20 moveType=SetMove (0) ...}	AdvancedMove
	▶ [7]	{from=22 to=22 moveType=SetMove (0) ...}	AdvancedMove
	▶ [Raw View]	{_Mypair=allocator }	std::vector<AdvancedMove,std::allocator>

4. Clarification Request on Readme.txt:

The original text of Readme.txt might have had some encoding issues, leading to garbled text upon opening. I have attempted a modification for clarity, but I am not entirely sure if my interpretation is accurate:

The program prints the evaluation of the current position in the bottom right corner (you can turn this off from the Settings). "TESC" roughly means that the position is deemed equal. What it actually means is that with perfect play (with regard to distinguished draws), the game will end in a draw where the players have an equal number of stones (that is, in a non-transient ESC subspace in the terminology of our paper). If it prints "L", you are losing (the second number in the parenthesis shows how many moves you can delay the loss at best). If it prints "W", you are winning. In other cases, it prints things like "-59 (std_8_9_0_0)". The four numbers identify the subspace in which the game will be drawn by perfect play: you will have 8 stones on the board, the computer will have 9, and none of you will have more stones to place. The first number (-59) is the value of the subspace. The center of the range of this value is 0, and a negative number means that you are in a bad situation. This number won’t increase since the program is playing perfectly. (If you make a mistake, then it decreases; otherwise, it stays the same.) For example, if the evaluation is 3_6_0_0, then you are very close to losing. The number after the "NGM:" shows the number of optimal moves in the current position.

Could you please provide a detailed explanation, especially regarding the evaluation results like NTESC, (18, 9)? I aim to judge the value of moves based on how many steps it takes to win, draw, or lose. Currently, I am only able to categorize moves as win, lose, or draw, which might not be very user-friendly.

Thank you very much for your time and assistance.

BTW. In the coming days, the Beta channel on Play Store will be updating the Sanmill App. This update will enable users to download and import the perfect database you developed, by following the guide at https://github.com/calcitem/Sanmill/wiki/Perfect-Database. When it's available, I would greatly appreciate it if you could check to see if the behavior meets expectations. Thank you very much for your time.

[ggevay]

"DD" means "distinguishing draws", which refers to our heuristic way to achieve an ultra-strong solution (as opposed to a strong solution): our retrograde analysis is based on the assumption that there are no turn limits, so a draw is actually a cycle or a combination of cycles in the graph of game states; we differentiate draws based on how many stones each player has in such a cycle.

For the meaning of the phrases "ultra-strong solution" and "strong solution", see our website, or our paper:
https://www.researchgate.net/publication/264459392_Calculating_Ultra-Strong_and_Extended_Solutions_for_Nine_Men's_Morris_Morabaraba_and_Lasker_Morris

// What's ignore_DD?

The ignore_DD variable means whether to turn off the ultra-strong solution, and just play strongly. (This is settable from our GUI.) In other words, when ignore_DD is true, then the program will not distinguish between draw positions.

I aim to judge the value of moves based on how many steps it takes to win, draw, or lose. Currently, I am only able to categorize moves as win, lose, or draw, which might not be very user-friendly.

To make the program’s move selection take into account how many steps it takes to win or lose (in the paper, DTW refers to this number of steps), you can simply use our comparison function. The “number of steps it takes to draw” is an ambiguous term; the definition we used in our program is a bit tricky, see our definition in subsection IV-B-2 of our paper.

For displaying the DTW to your users, you could look at key2 of gui_eval_elem2: In case of a win or a loss, key2 will simply be the DTW. In case of a draw, key2 is defined in a complicated way, as explained in our paper. This definition is not necessarily meaningful for users, so consider not displaying it.

(If the gameOptions.getSkillLevel() < 15 part in your code is intended to not differentiate among draws, we recommend using our ignore_DD code path instead. The code that runs when ignore_DD is true has the advantage that it takes the DTW of wins and losses into account. So, instead of adding an if in allMaxBy, you could just modify the if condition in the compare function to not check ignore_DD but check gameOptions.getSkillLevel() < 15 instead.)

The code in the commit calcitem/Sanmill@b906d31 has a bug in the compare function: There are two identical lines:

return key2 == o.key2 ? 0 : (key2 < o.key2 ? -1 : 1);

The second one should be negated. (Note that the corresponding code in your GitHub comment is correct.)

Note that the commit message of calcitem/Sanmill@b906d31 seems to suggest that the ultra-strong solution strives to win by as large stone difference as possible, but this is not the case. The ultra-strong solution differs from the strong solution only for draw positions. For wins and losses, the strong solution makes the same moves as the ultra-strong solution.

the meaning of NTESC, (18, 9) in the evaluation. My current understanding is that while all moves leading to a draw might be considered equal, only eight positions result in both players having the same number of stones, making them the most optimal. Other scenarios, although also resulting in a draw, do not meet this criterion of stone equality, hence are not considered optimal.

Yes, this is correct for the considered state, because stone advantage cannot be achieved in that state.

[calcitem]

Morabaraba occasional draws

Calcitem:

Hello, it seems that Malom's Perfect DB is not compatible with Morabaraba's extra rule. In addition, I found a problem. For Morabaraba's self-play results, DB does not always win the white side, but sometimes the game ends in a draw. I will explain the issue in detail through GitHub later.

GG:

Yes, the extra rule would have made the database much bigger, so we decided to not implement it. But we might still implement it at some point, as it's also an interesting question how the extra rule would influence perfect play. Also, today's computers could easily handle even the bigger database.
But not always winning with white in self play is quite concerning. We'll look into that. If you could provide more details on GitHub that would be great. In particular, does this happen when playing with our GUI, or when playing with your program with our database as the backend?

Regarding the issue of occasional draws in self-play mode, I have successfully replicated the issue, and the details are as follows:

Reproduction steps:
In Sanmill, copy the files from the Morabaraba_FBD_DD directory into the strong directory.

Configure as follows:

General Settings:

• Difficulty level: 1
• AI thinking time: 0
• Algorithm: Random
• Use perfect database: ON
• Move randomly: ON

Rule Settings:

• Threefold repetition rule: OFF

Switch to AI Vs AI mode and start a self-play game. The game resulted in a draw:

 1.    f2    f6
 2.    b2    d2
 3.    c5    b6
 4.    d6    g1
 5.    d5    d7
 6.    e5xd2    d2
 7.    e4    a7
 8.    e3xg1    g1
 9.    c3    g7xc3
10.    g4    d3
11.    a1    f4
12.    d1    c4
13.    a1-a4    c4-b4
14.    a4-a1    b4-c4
15.    a1-a4    c4-b4
16.    a4-a1    b4-c4
17.    a1-a4    c4-b4
18.    a4-a1    b4-c4
19.    a1-a4    c4-b4
20.    a4-a1    b4-c4
21.    a1-a4    c4-b4
22.    a4-a1    b4-c4
23.    a1-a4    c4-b4
24.    a4-a1    b4-c4
25.    a1-a4    c4-b4
26.    a4-a1    b4-c4
27.    a1-a4    c4-b4
28.    a4-a1    b4-c4
29.    a1-a4    c4-b4
30.    a4-a1    b4-c4
31.    a1-a4    c4-b4
32.    a4-a1    b4-c4
33.    a1-a4    c4-b4
34.    a4-a1    b4-c4
35.    a1-a4    c4-b4
36.    a4-a1    b4-c4
37.    a1-a4    c4-b4
38.    a4-a1    b4-c4
39.    a1-a4    c4-b4
40.    a4-a1    b4-c4
41.    a1-a4    c4-b4
42.    a4-a1    b4-c4
43.    a1-a4    c4-b4
44.    a4-a1    b4-c4
45.    a1-a4    c4-b4
46.    a4-a1    b4-c4
47.    a1-a4    c4-b4
48.    a4-a1    b4-c4
49.    a1-a4    c4-b4
50.    a4-a1    b4-c4
51.    a1-a4    c4-b4
52.    a4-a1    b4-c4
53.    a1-a4    c4-b4
54.    a4-a1    b4-c4
55.    a1-a4    c4-b4
56.    a4-a1    b4-c4
57.    a1-a4    c4-b4
58.    a4-a1    b4-c4
59.    a1-a4    c4-b4
60.    a4-a1    b4-c4
61.    a1-a4    c4-b4
62.    a4-a1    b4-c4

A simpler way to reproduce the draw is:

Copy the following content to the clipboard.

 1.    f2    f6
 2.    b2    d2
 3.    c5    b6
 4.    d6    g1
 5.    d5    d7
 6.    e5xd2    d2
 7.    e4    a7
 8.    e3xg1    g1
 9.    c3    g7xc3
10.    g4    d3
11.    a1    f4
12.    d1    c4
13.    a1-a4    c4-b4

Then click:

Game -> Import game

Click "Move -> Move now"

We will see a draw occur.

In https://github.com/calcitem/Sanmill/blob/master/src/perfect/perfect_common.h, we can see on Line 44:

#define DD // distinguish draws (ultra) //comment or uncomment

With the DD switch turned on, it should recognize a draw.

Open _Melom.exe,

Settings -> AI -> Ignore draw distinguishing info in the databases: OFF

Players -> First: Human

Players -> Second: Human

It is found that in the 12th round, the move returned by Perfect Database is d1, but the correct move should be c3 as illustrated below:

This indicates there might be a bug in the code adaptation, but I am unsure which part of the code should be checked first or how to debug it. Could you please provide some guidance? Thank you!