What's wrong with this code. In case of timeout bestMove is skipped in the caller and iterative deepening loop is ended. Also bestMove and it's value is only set when not timeout has occurred. So I don't understand why it sometimes selects a really bad move.
public int SearchRoot(int depth, int initVal, out MoveBase bestMove)
{
int WINDOW_MARGIN = 2000;
int factor = 1;
int lb = initVal - factor * WINDOW_MARGIN;
int ub = initVal + factor * WINDOW_MARGIN;
int value = Search(depth, lb, ub, out bestMove, false); ;
if (Expired())
throw new TimeoutException();
factor = factor * 4;
do
{
while (lb >= value)
{
lb = initVal - factor * WINDOW_MARGIN;
value = Search(depth, lb, ub, out bestMove, false);
if (Expired())
throw new TimeoutException();
factor = factor * 4;
}
while (ub <= value)
{
ub = initVal + factor * WINDOW_MARGIN;
value = Search(depth, lb, ub, out bestMove, false);
if (Expired())
throw new TimeoutException();
factor = factor * 4;
}
}
while (!(value > lb && value < ub));
return value;
}
I don't know but i see that you raise factor while searching for the lower bound, then you use the raised factor to start upper-bound loop. Maybe it is better to reset factor to 1 before starting "while(ub<=value)"? maybe you need to reset "value" too?
stegemma wrote:I don't know but i see that you raise factor while searching for the lower bound, then you use the raised factor to start upper-bound loop. Maybe it is better to reset factor to 1 before starting "while(ub<=value)"? maybe you need to reset "value" too?
Yes I can also try reset factor. Don't understand why I should reset value.
Might be that transposition table is causing these simple blunders. I don't know. Overwriting good entries with bad ones for their search is not completed ??
Take care that when you raise exception the function return value would be ignored by the caller. If you set the bestmove, even those value could be ignored, depending on how it is written the caller.
PS: it depends on how you write the catch part of the caller
You should not write results of incomplete (aborted) searches in the TT. In my engines I have a global abortFlag, which is tested just after the UnMake() after a recursive call, (i.e. inside the loop over moves), and when set causes the Search() routine to return immediately, giving it no chance to reach the part where it would start writing the TT.
'bestMove' keeps storing the value of the previous iteration, so that if the current iteration was aborted without having successfully searched any moves, it will return the move of the iteration that was finished. As this is also the move that is being searched first, there is no problem if only a single move is searched: you will still get the same move. Except that it then has the score from the new iteration instead of the previous one.
hgm wrote:You should not write results of incomplete (aborted) searches in the TT. In my engines I have a global abortFlag, which is tested just after the UnMake() after a recursive call, (i.e. inside the loop over moves), and when set causes the Search() routine to return immediately, giving it no chance to reach the part where it would start writing the TT.
'bestMove' keeps storing the value of the previous iteration, so that if the current iteration was aborted without having successfully searched any moves, it will return the move of the iteration that was finished. As this is also the move that is being searched first, there is no problem if only a single move is searched: you will still get the same move. Except that it then has the score from the new iteration instead of the previous one.
I only write in TT table when not time out. Don't know if writing TT entries with small windows cause trouble.
Well, it shouldn't. Having the window smaller than it really is (as in aspiration) would just make it more likely that an existing entry would have an acceptable bound, and would cause a hash cutoff rather than being re-searched. And if the bound was not good enough, it would certainly not have been good enough for the real window, so that it would have been re-searched in any case. It is just that after the re-search it gets a less tight bound with the smaller window, which might not be good enough next time when you search with a wider window because the aspiration failed. But that is of course why you aspirated in the first place, to save time by doing a more sloppy search.
If you think aspiration is related to the cause of some blunders your engine plays, just disable the aspiration code, to make sure that makes the blunders go away. If they don't, you would just be wasting your time looking at the aspiration. Aspiration gives you very few Elo anyway. At the level of Skipper you should be looking for fixes that improve it by more than a hundred Elo, and not make it difficult to find those by complex subtleties that only bring 10 Elo or so.
hgm wrote:Well, it shouldn't. Having the window smaller than it really is (as in aspiration) would just make it more likely that an existing entry would have an acceptable bound, and would cause a hash cutoff rather than being re-searched. And if the bound was not good enough, it would certainly not have been good enough for the real window, so that it would have been re-searched in any case. It is just that after the re-search it gets a less tight bound with the smaller window, which might not be good enough next time when you search with a wider window because the aspiration failed. But that is of course why you aspirated in the first place, to save time by doing a more sloppy search.
If you think aspiration is related to the cause of some blunders your engine plays, just disable the aspiration code, to make sure that makes the blunders go away. If they don't, you would just be wasting your time looking at the aspiration. Aspiration gives you very few Elo anyway. At the level of Skipper you should be looking for fixes that improve it by more than a hundred Elo, and not make it difficult to find those by complex subtleties that only bring 10 Elo or so.
An example of such a blunder (aspiration enabled). Skipper with black. I haven't seen this kind of blunders when aspiration is disabled.
[pgn]
1. e2e4 Nc6 2. Nf3 Nf6 3. e4e5
Nd5 4. c2c4 Nb6 5. d2d4 e7e6
6. d4d5 e6d5 7. c4d5 Bb4 8. Nc3
Ne7 9. d5d6 Bc3 10. b2c3 c7d6
11. e5d6 f7f5
[/pgn]
public int SearchRoot2(int depth, int initVal, out MoveBase bestMove)
{
int value = Search(depth, -INFINITE_INT, INFINITE_INT, out bestMove, false);
if (Expired())
throw new TimeoutException();
return value;
}
stegemma wrote:Take care that when you raise exception the function return value would be ignored by the caller. If you set the bestmove, even those value could be ignored, depending on how it is written the caller.
PS: it depends on how you write the catch part of the caller
