Cleanup TimeManagement - Unnecessarily min,max & clamp executions everytime #5052
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Can you remove the unrelated makefile change please and add yourself to the authors? |
Renaming doubleExtensions variable to multiExtensions, since now we have also triple extensions. Some extra cleanups. Recent tests used to measure the elo worth: https://tests.stockfishchess.org/tests/view/659fd0c379aa8af82b96abc3 https://tests.stockfishchess.org/tests/view/65a8f3da79aa8af82b9751e3 https://tests.stockfishchess.org/tests/view/65b51824c865510db0272740 https://tests.stockfishchess.org/tests/view/65b58fbfc865510db0272f5b closes official-stockfish#5032 No functional change
This refactors the CI workflows to group some logic and makes sure that all (pre)release binaries are actually tested. The screenshot below shows the execution logic of the reworked ci, https://github.com/Disservin/Stockfish/actions/runs/7773581379. You can also hover over the cards to see the execution flow. The `matrix.json` and `arm_matrix.json` define the binaries which will be uploaded to GitHub. Afterwards a matrix is created and each job compiles a profile guided build for that arch and uploads that as an artifact to GitHub. The Binaries/ARM_Binaries workflow's are called when the previous step has been completed, and uploads all artifacts to the (pre)release. This also fixes some indentations and renames the workflows, see https://github.com/official-stockfish/Stockfish/actions, where every workflow is called `Stockfish` vs https://github.com/Disservin/Stockfish/actions. It also increases the parallel compilation used for make from `-j2 to -j4`. It now also prevents the prerelease action from running on forks. A test release can be viewed here https://github.com/Disservin/Stockfish/releases. closes official-stockfish#5035 No functional change
Move divisor from capture scoring to good capture check and sligthly increase it. This has several effects: - its a speedup because for quience and probcut search the division now never happens. For main search its delayed and can be avoided if a good capture triggers a cutoff - through the higher resolution of scores we have a more granular sorting STC: https://tests.stockfishchess.org/tests/view/65bf2a93c865510db027dc27 LLR: 2.93 (-2.94,2.94) <0.00,2.00> Total: 470016 W: 122150 L: 121173 D: 226693 Ptnml(0-2): 2133, 55705, 118374, 56644, 2152 LTC: https://tests.stockfishchess.org/tests/view/65c1d16dc865510db0281339 LLR: 2.96 (-2.94,2.94) <0.50,2.50> Total: 98988 W: 25121 L: 24667 D: 49200 Ptnml(0-2): 77, 10998, 26884, 11464, 71 closes official-stockfish#5036 Bench: 1233867
With the recent introduction of the dual NNUE, the need for simple eval is no longer there. Passed STC: https://tests.stockfishchess.org/tests/view/65c1f735c865510db0281652 LLR: 2.96 (-2.94,2.94) <-1.75,0.25> Total: 85312 W: 22009 L: 21837 D: 41466 Ptnml(0-2): 334, 10155, 21567, 10205, 395 Passed LTC: https://tests.stockfishchess.org/tests/view/65c2d64bc865510db0282810 LLR: 2.95 (-2.94,2.94) <-1.75,0.25> Total: 49956 W: 12596 L: 12402 D: 24958 Ptnml(0-2): 28, 5553, 13624, 5743, 30 closes official-stockfish#5037 Bench 1213676
This removes the reduction decrease that occured when the previous ply had a movecount greater than 7. Passed STC: https://tests.stockfishchess.org/tests/view/65c3f6dac865510db0283ef6 LLR: 2.94 (-2.94,2.94) <-1.75,0.25> Total: 11968 W: 3205 L: 2953 D: 5810 Ptnml(0-2): 38, 1310, 3064, 1506, 66 Passed LTC: https://tests.stockfishchess.org/tests/view/65c42377c865510db0284217 LLR: 2.94 (-2.94,2.94) <-1.75,0.25> Total: 35676 W: 9113 L: 8905 D: 17658 Ptnml(0-2): 22, 3893, 9802, 4097, 24 closes official-stockfish#5040 Bench: 1148379
Passed simplification STC: https://tests.stockfishchess.org/tests/view/65c38d2ac865510db02836cf LLR: 2.93 (-2.94,2.94) <-1.75,0.25> Total: 52288 W: 13578 L: 13371 D: 25339 Ptnml(0-2): 197, 6171, 13265, 6250, 261 Passed simplification LTC: https://tests.stockfishchess.org/tests/view/65c4470ec865510db0284473 LLR: 2.95 (-2.94,2.94) <-1.75,0.25> Total: 44958 W: 11255 L: 11055 D: 22648 Ptnml(0-2): 37, 4962, 12274, 5176, 30 closes official-stockfish#5041 Bench: 1116591
Fixes the issue mentioned in official-stockfish@584d9ef#r138417600. Thanks to @cj5716 and @peregrineshahin for spotting this! closes official-stockfish#5042 No functional change
- Update codeql to v3 - Switch from dev-drprasad to native github cli - Update softprops/action-gh-release to node 20 commit `thollander/actions-comment-pull-request` needs to be bumped to node20 too, but the author hasnt done so atm closes official-stockfish#5044 No functional change
In both search and qsearch, there are instances where we do unadjustedStaticEval = ss->staticEval = eval/bestValue = tte->eval(), but immediately after re-assign ss-static and eval/bestValue to some new value, which makes the initial assignment redundant. closes official-stockfish#5045 No functional change
Assorted cleanups closes official-stockfish#5046 No functional change Co-Authored-By: Shahin M. Shahin <41402573+peregrineshahin@users.noreply.github.com> Co-Authored-By: cj5716 <125858804+cj5716@users.noreply.github.com>
This patch does similar thing to how it's done for qsearch - in case of fail high adjust result to lower value. Difference is that it is done only for non-pv nodes and it's depth dependent - so lower depth entries will have bigger adjustment and higher depth entries will have smaller adjustment. Passed STC: https://tests.stockfishchess.org/tests/view/65c3c0cbc865510db0283b21 LLR: 2.96 (-2.94,2.94) <0.00,2.00> Total: 112032 W: 29142 L: 28705 D: 54185 Ptnml(0-2): 479, 13152, 28326, 13571, 488 Passed LTC: https://tests.stockfishchess.org/tests/view/65c52e62c865510db02855d5 LLR: 2.96 (-2.94,2.94) <0.50,2.50> Total: 132480 W: 33457 L: 32936 D: 66087 Ptnml(0-2): 67, 14697, 36222, 15156, 98 closes official-stockfish#5047 Bench: 1168241
Initialize the unordered map to a reasonable number of buckets and make the move hashes well distributed. For more see official-stockfish#4958 (comment) Also make bestThreadPV and newThreadPV references so we don't copy entire vectors. closes official-stockfish#5048 No functional change
Passed STC: https://tests.stockfishchess.org/tests/view/65c6934cc865510db0286e90 LLR: 2.99 (-2.94,2.94) <-1.75,0.25> Total: 54016 W: 14065 L: 13854 D: 26097 Ptnml(0-2): 231, 6381, 13581, 6576, 239 Passed LTC: https://tests.stockfishchess.org/tests/view/65c72b91c865510db0287a1a LLR: 2.96 (-2.94,2.94) <-1.75,0.25> Total: 55098 W: 13850 L: 13658 D: 27590 Ptnml(0-2): 37, 6257, 14777, 6433, 45 closes official-stockfish#5049 Bench: 1027182
No functional change
|
Uff you kinda made it worse :D I'll fix when merging, maybe those will help in the future.. https://christoph-rumpel.com/2015/05/clean-up-your-commits-for-a-pull-request |
|
haha yeah i was trying to fix it, sorry still learning git. ill stop messing around with this branch then |
xu-shawn
pushed a commit
to xu-shawn/Stockfish
that referenced
this pull request
Feb 15, 2024
Move optExtra, optConstant and maxConstant into lower scope. closes official-stockfish#5052 No functional change
xu-shawn
pushed a commit
to xu-shawn/Stockfish
that referenced
this pull request
Feb 19, 2024
Move optExtra, optConstant and maxConstant into lower scope. closes official-stockfish#5052 No functional change
xu-shawn
pushed a commit
to xu-shawn/Stockfish
that referenced
this pull request
Feb 19, 2024
Move optExtra, optConstant and maxConstant into lower scope. closes official-stockfish#5052 No functional change
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
three calculations were being executed unecessarily everytime SF was calculating maximumTime and optimumTime.
They are only required in the scenario that movesToGo == 0.
moved these calculations to within the appropriate IF statement.
Non Functional Change, could potentially save some small amount of calculation time every move though.