Skip to content
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

Eliminate ONE_PLY #2289

Closed

Conversation

SapphireBrand
Copy link

Simplification that eliminates ONE_PLY, based on a suggestion in the forum that support for fractional plies has never been used, and @mcostalba's openness to the idea of eliminating it.

Binary identical to master.

No functional change

@vondele
Copy link
Member

vondele commented Sep 2, 2019

missing bit:

diff --git a/.travis.yml b/.travis.yml
index 1d56a23e9..a59ea4251 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -56,9 +56,6 @@ script:
   - make clean && make -j2 ARCH=x86-32 optimize=no debug=yes build && ../tests/signature.sh $benchref
   - make clean && make -j2 ARCH=x86-32 build && ../tests/signature.sh $benchref
 
-  # Verify bench number is ONE_PLY independent by doubling its value
-  - sed -i'.bak' 's/.*\(ONE_PLY = [0-9]*\),.*/\1 * 2,/g' types.h
-  - make clean && make -j2 ARCH=x86-64 build && ../tests/signature.sh $benchref
   #
   # Check perft and reproducible search
   - ../tests/perft.sh

@Vizvezdenec
Copy link
Contributor

Well I do agree.
All functions that use depth divided by smth look extremely ugly, especially if you use std::min
Like std::min(((depth - 4 * ONE_PLY) / ONE_PLY) / 2, 0) * ONE_PLY etc
For the sake of potential functionality that was never used?

@locutus2
Copy link
Member

locutus2 commented Sep 2, 2019

I have tried int the past several tests around fractional plies but had no success. But i'am personally convinced that with fractional plies there a higher optimum is possible. I understand that removing simplifies the code and coding (which for me makes no really difference, i never forget the ONE_PLY, its a automatic move for me) but if we remove it nobody will test around fractional plies anymore because too much would be to changed for it (and probably something is forgotten and an error is introduced).

Perhaps we should set a target date and call for a period of intensive fractional plies testing. And if nothing is found until then simplify it away.

@gvreuls
Copy link
Contributor

gvreuls commented Sep 2, 2019

I've been playing around with fractional plies since #2081 and haven't come up with any results either. To my understanding factional plies as they're implemented (with the tt only saving integer plies) can only be used for search reductions and there they're just a way to do less search reduction (skip every other half ply reduction), so they're less efficient than full ply reductions in almost every instance (because they search deeper).

@vondele
Copy link
Member

vondele commented Sep 2, 2019

The first request I can find to keep ONE_PLY 'a few more months' was in 2016:
#916 (comment)
similar request in 2018:
#1543 (comment)
We've kept it around for a few years without impact on performance... time to let it go IMO.

@MichaelB7
Copy link
Contributor

@gvreuls They also could be used for extensions. For every other ply as noted in your comment regarding reductions or having multiple factors existing on a move , that would trigger a full ply extension.

@MichaelB7
Copy link
Contributor

I would suggest a 6 month moratorium before removing it to allow one more shot at it with testing - if none is found, remove it. But since we know it doesn’t hurt the performance or current testing , my first choice would be to leave it “as is” since any re-introduction at a later time could be problematic.

@xoto10
Copy link
Contributor

xoto10 commented Sep 2, 2019

We could try tuning the related variables with ONE_PLY set to something like 256 and see what comes out?

One issue is there are at least a couple of places where the * ONE_PLY is too late in the order of execution to use the full significance (see tests by sg), e.g.
r -= ss->statScore / 16384 * ONE_PLY;
and
return ((r + 520) / 1024 + (!i && r > 999)) * ONE_PLY;

Perhaps there are others?

@SapphireBrand
Copy link
Author

To some extent, this comes down to "enough people have tried and failed to make fractional search work that we have decided as a community to focus our attention on other avenues, which have proved much more fruitful."

In addition to having entirely failed in our experience, there are theoretical reasons why fractional search depths should be difficult to make work:

  1. TT stores depth as an integer, whereas fractional depths will have a lot of "in between" values. E.g., you stored a 5-ply result, but now you are doing a 5.125 ply search, so...a lot of positions no longer match.
  2. Depth cutoffs are built into every major system in search. There are a lot of synergistic and counter-balancing effects from null move, futility, iterative deepening, LMR, singular,... It seems likely to me that the balance will be upset by fractional depths. That is, there are implicit assumptions that a move is 1 ply.
  3. LMR already gives the program the effect of fractional depth, just with a different frame of reference. That is, depth decrements in LMR have all sorts of ratios of non-integer ratios, but without any fractions.

To summarize, fractional depth systems

  • haven't worked, despite a lot of engineers trying to find such things (me too),
  • will always reduce efficiency in the TT,
  • are likely to disrupt other systems (making them difficult to find/test/evaluate), and
  • are probably not much better than an LMR change, which has proven to be easier to find.

@xoto10
Copy link
Contributor

xoto10 commented Sep 3, 2019 via email

@locutus2
Copy link
Member

locutus2 commented Sep 4, 2019

@SapphireBrand
That are good points which makes it difficult to make fractional plies working. Especially we have the problem that for a minimum fractional ply change several uses of depth changes the outcome.
Par example for LMR we have the condition depth >= 3 * ONE_PLY. If ins master depth = 3 and its now only slightly reduced by a fractional ply this condition doesn't trigger anymore. To avoid this sharp changes i implemented a branch (using ONE_PLY=256) which adjusts the depth by a half ply so that the trigger interval is centered around the full plies and not sit at the boundaries.
https://github.com/locutus2/Stockfish/tree/fractional_base
The bench shouldn't change but it does so i obviously i have overlooked something or adjusted in the wrong way. So if someone finds some of my errors feedback would be nice :-)
Nevertheless i will try further tests based on above base branch.

Edit:
On the last point in your arguments i have to disagree. If we use par example ONE_PLY=256 the parameter space is increases by the factor 256. So it seems very unlikely that the optimal setting is in the subspace of ONE_PLY=1. The problem is to find it.

locutus2 added a commit to locutus2/Stockfish-old that referenced this pull request Sep 4, 2019
…ased on my a adjusted fractional_base branch (see also official-stockfish/Stockfish#2289). Bench: 3504904
@snicolet
Copy link
Member

What date in the future should we decide for the removal, if no test proves that the ONE_PLY feature is useful after all?

@Vizvezdenec
Copy link
Contributor

I think that everyone who wanted already tried this partial plies. My guess that today will be pretty OK.

@SapphireBrand
Copy link
Author

I will make a fresh version on top of the latest master, so we can all take another look.

@SapphireBrand
Copy link
Author

SapphireBrand commented Sep 28, 2019

@SapphireBrand
On the last point in your arguments i have to disagree. If we use par example ONE_PLY=256 the
parameter space is increases by the factor 256. So it seems very unlikely that the optimal setting is in the subspace of ONE_PLY=1. The problem is to find it.

@locutus2
The dimensionality argument is plausible if Stockfish development were starting from scratch, provided that someone invents a way to overcome inherent xref inefficiency. But starting from here, it seems likely (to me) that ONE_PLY == 1 is a deeply entrenched local optimum. I cited theoretical reasons for this, and the experiments of quite a few devoted researchers (including yourself) seem to confirm this.

That is, I think that you have found the optimum, and, furthermore, it is through your efforts that the community has that confidence.

@snicolet
Copy link
Member

@SapphireBrand
Could you edit the PR message, adding references and links to the various discussions we had about the ONE_PLY feature? thanks :-)

…forum that support for fractional plies has never been used, and @mcostalba's openness to the idea of eliminating it.

The argument favoring eliminating ONE_PLY:
* The term “ONE_PLY” comes up in a lot of forum posts (https://groups.google.com/forum/?fromgroups=#!searchin/fishcooking/ONE_PLY%7Csort:relevance): 474 posts to date.
* There is occasionally a commit that breaks invariance of the code with respect to ONE_PLY. (https://groups.google.com/forum/?fromgroups=#!searchin/fishcooking/ONE_PLY%7Csort:date/fishcooking/ZIPdYj6k0fk/KdNGcPWeBgAJ)
* To prevent such commits, there is a Travis CI hack that doubles ONE_PLY and rechecks bench.
* Sustaining ONE_PLY has, alas, not resulted in any improvements to the engine, despite many individuals testing many experiments over 5 years.

The strongest argument in favor of preserving ONE_PLY comes from @locutus: “If we use par example ONE_PLY=256 the parameter space is increases by the factor 256. So it seems very unlikely that the optimal setting is in the subspace of ONE_PLY=1.”

There is a strong theoretical impediment to fractional depth systems: the transposition table uses depth to determine when a stored result is good enough to supply an answer for a current search. If you have fractional depths, then different pathways to the position can be at fractionally different depths.

In the end, there are 3 separate times when a proposal to remove ONE_PY was defeated by the suggestion to “give it a few more months.” So… it seems like time to remove this distraction from the community.

Binary identical to master.

No functional change
@SapphireBrand
Copy link
Author

Rebased onto the latest master, and updated the commit comment to include a summary (necessarily abbreviated) of the discussion.

@vondele
Copy link
Member

vondele commented Oct 3, 2019

@snicolet this should be merged, as maintaining the PR is tedious, every other commit to master causes a conflict.

snicolet pushed a commit that referenced this pull request Oct 5, 2019
Simplification that eliminates ONE_PLY, based on a suggestion in the forum that
support for fractional plies has never been used, and @mcostalba's openness to
the idea of eliminating it. We lose a little bit of type safety by making Depth
an integer, but in return we simplify the code in search.cpp quite significantly.

No functional change

------------------------------------------

The argument favoring eliminating ONE_PLY:

* The term “ONE_PLY” comes up in a lot of forum posts (474 to date)
https://groups.google.com/forum/?fromgroups=#!searchin/fishcooking/ONE_PLY%7Csort:relevance

* There is occasionally a commit that breaks invariance of the code
with respect to ONE_PLY
https://groups.google.com/forum/?fromgroups=#!searchin/fishcooking/ONE_PLY%7Csort:date/fishcooking/ZIPdYj6k0fk/KdNGcPWeBgAJ

* To prevent such commits, there is a Travis CI hack that doubles ONE_PLY
and rechecks bench

* Sustaining ONE_PLY has, alas, not resulted in any improvements to the
  engine, despite many individuals testing many experiments over 5 years.

The strongest argument in favor of preserving ONE_PLY comes from @locutus:
“If we use par example ONE_PLY=256 the parameter space is increases by the
factor 256. So it seems very unlikely that the optimal setting is in the
subspace of ONE_PLY=1.”

There is a strong theoretical impediment to fractional depth systems: the
transposition table uses depth to determine when a stored result is good
enough to supply an answer for a current search. If you have fractional
depths, then different pathways to the position can be at fractionally
different depths.

In the end, there are three separate times when a proposal to remove ONE_PLY
was defeated by the suggestion to “give it a few more months.” So… it seems
like time to remove this distraction from the community.

See the pull request here:
#2289
@snicolet
Copy link
Member

snicolet commented Oct 5, 2019

Merged via ca7d4e9, thanks!

We lose a little bit of type safety by making Depth an integer, but in return we simplify the code in search.cpp quite significantly.

@snicolet snicolet closed this Oct 5, 2019
MichaelB7 pushed a commit to MichaelB7/Stockfish that referenced this pull request Oct 8, 2019
Simplification that eliminates ONE_PLY, based on a suggestion in the forum that
support for fractional plies has never been used, and @mcostalba's openness to
the idea of eliminating it. We lose a little bit of type safety by making Depth
an integer, but in return we simplify the code in search.cpp quite significantly.

No functional change

------------------------------------------

The argument favoring eliminating ONE_PLY:

* The term “ONE_PLY” comes up in a lot of forum posts (474 to date)
https://groups.google.com/forum/?fromgroups=#!searchin/fishcooking/ONE_PLY%7Csort:relevance

* There is occasionally a commit that breaks invariance of the code
with respect to ONE_PLY
https://groups.google.com/forum/?fromgroups=#!searchin/fishcooking/ONE_PLY%7Csort:date/fishcooking/ZIPdYj6k0fk/KdNGcPWeBgAJ

* To prevent such commits, there is a Travis CI hack that doubles ONE_PLY
and rechecks bench

* Sustaining ONE_PLY has, alas, not resulted in any improvements to the
  engine, despite many individuals testing many experiments over 5 years.

The strongest argument in favor of preserving ONE_PLY comes from @locutus:
“If we use par example ONE_PLY=256 the parameter space is increases by the
factor 256. So it seems very unlikely that the optimal setting is in the
subspace of ONE_PLY=1.”

There is a strong theoretical impediment to fractional depth systems: the
transposition table uses depth to determine when a stored result is good
enough to supply an answer for a current search. If you have fractional
depths, then different pathways to the position can be at fractionally
different depths.

In the end, there are three separate times when a proposal to remove ONE_PLY
was defeated by the suggestion to “give it a few more months.” So… it seems
like time to remove this distraction from the community.

See the pull request here:
official-stockfish#2289
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants