Stockfish Nullmove uci option #178

[phenri]

Hello,

I would like your opinion on this patch https://github.com/mcostalba/Stockfish/pull/178/files
It is supposed to disable Nullmove

The patch replace this

Code: Select all

if (   !PvNode
        && !ss->skipNullMove
        &&  depth >= 2 * ONE_PLY
        &&  eval >= beta
        &&  abs(beta) < VALUE_MATE_IN_MAX_PLY
        &&  pos.non_pawn_material(pos.side_to_move()))
    {

by this:

Code: Select all

if (    Options["Enable Null Move"]
        && !PvNode
        && !ss->skipNullMove
        &&  depth >= 2 * ONE_PLY
        &&  eval >= beta
        &&  abs(beta) < VALUE_MATE_IN_MAX_PLY
        &&  pos.non_pawn_material(pos.side_to_move()))
    {

And I propose the following: (replace && by ||) Is this correct or is it more complex than that?

Code: Select all

if (    Options["Enable Null Move"]
        || !PvNode
        && !ss->skipNullMove
        &&  depth >= 2 * ONE_PLY
        &&  eval >= beta
        &&  abs(beta) < VALUE_MATE_IN_MAX_PLY
        &&  pos.non_pawn_material(pos.side_to_move()))
    {

[Evert]

Using a hash in search to look up an option setting looks inefficient to me. I would copy the setting out to a variable at the start of the search, or (actually) just when the value changes.

Other than that

And I propose the following: (replace && by ||) Is this correct or is it more complex than that?

Looks wrong to me, unless your intention is to force the code to always try a null move (even if it normally would not). But I would think that the intended behaviour is to be able to disable the null move, right? In that case, the first version is what you want (try null move if it is enabled and conditions are such that you want to give it a shot).

[phenri]

Thanks

I forget to mention this change:

Code: Select all

  o["Enable Null Move"]            = Option(true);

to

Code: Select all

  o["Disable Null Move"]            = Option(false);

My intention was to change "IF ... AND ... AND ..." to "IF Options["Disable Null Move"] ... OR IF !PvNode && ..."

Edit:
I mean if the option is set to "FALSE" then Stockfish behaves like the original, and if it is "TRUE" that activates the patch supposed to disable "Nullmove".

Is that it makes sense to you?

[hwiechers]

Marco will never accept a patch that disables null move.
He's also against adding unnecessary UCI options.

If you really want a variant of Stockfish without null move you should just fork it and and just delete the null move code.
This will probably be easier than adding the option and the end product will be more efficient as well.

[phenri]

Thank you, but it was not the subject of my question.

For indeed I already know that Marco is against any kind of thing that could make life easier for players correspondence, as well as for those who analyze their games.
If the best of chess, such Houdini engines was a donkey. His only interest in Stockfish would be to have the role of the stick.

But okay, back to the question, what then is the part of the code that should remove?

[syzygy]

Thank you, but it was not the subject of my question.

But it is the answer to the question. You have the code, so you can just do it yourself.

Regarding your patch, invoking Options() within search() is clearly an absolute no no no. The SF code gives numerous examples of how to do this properly.

If you don't know the precise meaning of && and || then maybe first look it up. Also consider using parentheses to make sure your intention is expressed correctly. Submitting guess work and hoping that Marco will take it more seriously is not going to work.

[phenri]

Thank you, but it was not the subject of my question.

But it is the answer to the question. You have the code, so you can just do it yourself.

Regarding your patch, invoking Options() within search() is clearly an absolute no no no. The SF code gives numerous examples of how to do this properly.

If you don't know the precise meaning of && and || then maybe first look it up. Also consider using parentheses to make sure your intention is expressed correctly. Submitting guess work and hoping that Marco will take it more seriously is not going to work.

And I do not pretend to make a patch. I just wanted to understand if the patch was correctly used in the state. And if not, then I was correct in changing the programming logic operators.

I have neither the time nor the inclination to cleansed your hateful to those who request assistance to programmers.
Know that we do not improvise programmer in 1 day or 1-year. Also note that if everyone can read and write, it does not mean that all have the skills to learn programming.

So spare me please your mood and those of Marco.

But I nonetheless appreciated your help. Thank you.

So for those who are willing to help me

Do you think now that it is correct and usable?

Code: Select all

if (    (Options["Disable Null Move"])
        || (!PvNode
        && !ss->skipNullMove
        &&  depth >= 2 * ONE_PLY
        &&  eval >= beta
        &&  abs(beta) < VALUE_MATE_IN_MAX_PLY
        &&  pos.non_pawn_material(pos.side_to_move())))
    {

[bob]

Thank you, but it was not the subject of my question.

But it is the answer to the question. You have the code, so you can just do it yourself.

Regarding your patch, invoking Options() within search() is clearly an absolute no no no. The SF code gives numerous examples of how to do this properly.

If you don't know the precise meaning of && and || then maybe first look it up. Also consider using parentheses to make sure your intention is expressed correctly. Submitting guess work and hoping that Marco will take it more seriously is not going to work.

And I do not pretend to make a patch. I just wanted to understand if the patch was correctly used in the state. And if not, then I was correct in changing the programming logic operators.

I have neither the time nor the inclination to cleansed your hateful to those who request assistance to programmers.
Know that we do not improvise programmer in 1 day or 1-year. Also note that if everyone can read and write, it does not mean that all have the skills to learn programming.

So spare me please your mood and those of Marco.

But I nonetheless appreciated your help. Thank you.

So for those who are willing to help me

Do you think now that it is correct and usable?

Code: Select all

if (    (Options["Disable Null Move"])
        || (!PvNode
        && !ss->skipNullMove
        &&  depth >= 2 * ONE_PLY
        &&  eval >= beta
        &&  abs(beta) < VALUE_MATE_IN_MAX_PLY
        &&  pos.non_pawn_material(pos.side_to_move())))
    {

Looks wrong. Here's why:

if ( (Options["Disable Null Move"]) -> if this is true (null-move is disabled) you are going to force null-move searches on EVERY node. I am assuming "Options(...) returns 1 if that option is set, 0 otherwise. That does NOT sound like what you want.

If that test returns false, then the usual do-null-criteria are used (not pv node, etc.)

I don't see why you changed the && to ||. It sounds more like you want to put the && back in, and change that to "enable null move". Then if enable is not set, no nulls are ever done. Going to be a big Elo loser, however.

|| (!PvNode
&& !ss->skipNullMove
&& depth >= 2 * ONE_PLY
&& eval >= beta
&& abs(beta) < VALUE_MATE_IN_MAX_PLY
&& pos.non_pawn_material(pos.side_to_move())))

[syzygy]

And I do not pretend to make a patch.

Oh sorry! I thought you had submitted patch #178, but now that I reread your post I probably got that wrong.

Know that we do not improvise programmer in 1 day or 1-year. Also note that if everyone can read and write, it does not mean that all have the skills to learn programming.

Sure, but I wrote my remark thinking you had submitted this patch to Marco.

The original patch is correct from a logical point of view. The "Options" should be replaced by a variable that is initialised before the search starts (for efficiency). I would keep the !PvNode test at the front, but that is not critical.

Your proposed change is not correct and to be honest I don't even know what it would do, because I am not sure whether || or && takes precedence. I also don't want to know, because this should be made clear in the code using parentheses. So either Options || (... && ... && ..) or (Options || !PvNode) && ... && ..., but neither does what you would want it to do.

[hgm]

Excessive use of parentheses is very bad for readability of code. Not knowing operator precedence is therefor a bad idea.