New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make razor_margin[4] ONE_PLY value independent #814

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@HiraokaTakuya

HiraokaTakuya commented Sep 28, 2016

No description provided.

@lucasart

This comment has been minimized.

Show comment
Hide comment
@lucasart

lucasart Sep 29, 2016

Patch is wrong: If ONE_PLY value is changed, you will read outside the bounds of the array.

Current code is already ONE_PLY independent, so I don't understand what you're trying to do.

lucasart commented Sep 29, 2016

Patch is wrong: If ONE_PLY value is changed, you will read outside the bounds of the array.

Current code is already ONE_PLY independent, so I don't understand what you're trying to do.

@HiraokaTakuya

This comment has been minimized.

Show comment
Hide comment
@HiraokaTakuya

HiraokaTakuya Sep 29, 2016

If ONE_PLY is 2, razor_margin[3 * ONE_PLY] is razor_margin[6].
It's outside the bounds of the array.
Why is current code is already ONE_PLY independent?

HiraokaTakuya commented Sep 29, 2016

If ONE_PLY is 2, razor_margin[3 * ONE_PLY] is razor_margin[6].
It's outside the bounds of the array.
Why is current code is already ONE_PLY independent?

@lucasart

This comment has been minimized.

Show comment
Hide comment
@lucasart

lucasart commented Sep 29, 2016

Why not ?

@HiraokaTakuya

This comment has been minimized.

Show comment
Hide comment
@HiraokaTakuya

HiraokaTakuya Sep 29, 2016

Current code : razor_margin[3 * ONE_PLY]
If ONE_PLY is 2, It's outside the bounds of the array.

My patch code : razor_margin[3]
If ONE_PLY is 2, It is "not" outside the bounds of the array.

HiraokaTakuya commented Sep 29, 2016

Current code : razor_margin[3 * ONE_PLY]
If ONE_PLY is 2, It's outside the bounds of the array.

My patch code : razor_margin[3]
If ONE_PLY is 2, It is "not" outside the bounds of the array.

@Kingdefender

This comment has been minimized.

Show comment
Hide comment
@Kingdefender

Kingdefender Sep 29, 2016

Hello,

This patch as proposed by Hiraoka Takuya seems correct to me.

As an aside, it does not seem very likely to me that Stockfish will go back to fractional plies smaller than a half move, so maybe the ONE_PLY constant could go altogether? It seems more likely that search could go to increasing by a full move, eventually.

Also the tuned Razor margin does not go up very clearly with depth, Maybe you could instead use here (I believe this is Michel van den Bergh's patch) the value 483, that is not so different from 554 what it is now. Then you could change the code to && eval + razor_margin[DEPTH_ZERO] <= alpha)

Or maybe this razoring margin should not be depth dependant but just something a bit larger than two pawns, and since search does not know what phase it is in, uses the larger valueof a pawn in the endgame plus a little bit for positional eval error. I have a suspicion, search could be made more dependant on phase because the number of meaningful captures changes during the game (in my opinion).

Eelco

Kingdefender commented Sep 29, 2016

Hello,

This patch as proposed by Hiraoka Takuya seems correct to me.

As an aside, it does not seem very likely to me that Stockfish will go back to fractional plies smaller than a half move, so maybe the ONE_PLY constant could go altogether? It seems more likely that search could go to increasing by a full move, eventually.

Also the tuned Razor margin does not go up very clearly with depth, Maybe you could instead use here (I believe this is Michel van den Bergh's patch) the value 483, that is not so different from 554 what it is now. Then you could change the code to && eval + razor_margin[DEPTH_ZERO] <= alpha)

Or maybe this razoring margin should not be depth dependant but just something a bit larger than two pawns, and since search does not know what phase it is in, uses the larger valueof a pawn in the endgame plus a little bit for positional eval error. I have a suspicion, search could be made more dependant on phase because the number of meaningful captures changes during the game (in my opinion).

Eelco

@mcostalba

This comment has been minimized.

Show comment
Hide comment
@mcostalba

mcostalba Sep 29, 2016

Merged with b77bae0.

BTW the condition is never triggered!

This patch surprised me because changing ONE_PLY value, I got teh same bench number even before the patch, OTH the patch is defenetly correct, so what was happening?

Indeed to my surprise the condition is always true 100% of cases! and for any value of the index, not only 3!!!

So I am going to remove that dead code.

mcostalba commented Sep 29, 2016

Merged with b77bae0.

BTW the condition is never triggered!

This patch surprised me because changing ONE_PLY value, I got teh same bench number even before the patch, OTH the patch is defenetly correct, so what was happening?

Indeed to my surprise the condition is always true 100% of cases! and for any value of the index, not only 3!!!

So I am going to remove that dead code.

@mcostalba mcostalba closed this Sep 29, 2016

@mcostalba

This comment has been minimized.

Show comment
Hide comment
@mcostalba

mcostalba commented Sep 29, 2016

See eccccba

@lucasart

This comment has been minimized.

Show comment
Hide comment
@lucasart

lucasart Sep 29, 2016

Sorry, I read the diff wrong! Indeed it was the master that had the 3 * ONE_PLY, which is wrong.

lucasart commented Sep 29, 2016

Sorry, I read the diff wrong! Indeed it was the master that had the 3 * ONE_PLY, which is wrong.

@HiraokaTakuya

This comment has been minimized.

Show comment
Hide comment
@HiraokaTakuya

HiraokaTakuya commented Sep 29, 2016

No problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment