Dual NNUE with L1-128 smallnet

[linrock]

Credit goes to @mstembera for:

• writing the code enabling dual NNUE: [RFC] [WIP] Dual net NNUE #4898
• the idea of trying L1-128 trained exclusively on high simple eval positions

The L1-128 smallnet is:

• epoch 399 of a single-stage training from scratch
• trained only on positions from filtered data with high material difference
  • defined by abs(simple_eval) > 1000

experiment-name: 128--S1-only-hse-v2

training-dataset:
  - /data/hse/S3/dfrc99-16tb7p-eval-filt-v2.min.high-simple-eval-1k.binpack
  - /data/hse/S3/leela96-filt-v2.min.high-simple-eval-1k.binpack
  - /data/hse/S3/test80-apr2022-16tb7p.min.high-simple-eval-1k.binpack

  - /data/hse/S7/test60-2020-2tb7p.v6-3072.high-simple-eval-1k.binpack
  - /data/hse/S7/test60-novdec2021-12tb7p-filter-v6-dd.min-mar2023.unmin.high-simple-eval-1k.binpack

  - /data/hse/S7/test77-nov2021-2tb7p.v6-3072.min.high-simple-eval-1k.binpack
  - /data/hse/S7/test77-dec2021-16tb7p-filter-v6-dd.min-mar2023.unmin.high-simple-eval-1k.binpack
  - /data/hse/S7/test77-jan2022-2tb7p.high-simple-eval-1k.binpack

  - /data/hse/S7/test78-jantomay2022-16tb7p-filter-v6-dd.min-mar2023.unmin.high-simple-eval-1k.binpack
  - /data/hse/S7/test78-juntosep2022-16tb7p-filter-v6-dd.min-mar2023.unmin.high-simple-eval-1k.binpack

  - /data/hse/S7/test79-apr2022-16tb7p-filter-v6-dd.min-mar2023.unmin.high-simple-eval-1k.binpack
  - /data/hse/S7/test79-may2022-16tb7p-filter-v6-dd.min-mar2023.unmin.high-simple-eval-1k.binpack

  # T80 2022
  - /data/hse/S7/test80-may2022-16tb7p.high-simple-eval-1k.binpack
  - /data/hse/S7/test80-jun2022-16tb7p-filter-v6-dd.min-mar2023.unmin.high-simple-eval-1k.binpack
  - /data/hse/S7/test80-jul2022-16tb7p.v6-dd.min.high-simple-eval-1k.binpack
  - /data/hse/S7/test80-aug2022-16tb7p-filter-v6-dd.min-mar2023.unmin.high-simple-eval-1k.binpack
  - /data/hse/S7/test80-sep2022-16tb7p-filter-v6-dd.min-mar2023.unmin.high-simple-eval-1k.binpack
  - /data/hse/S7/test80-oct2022-16tb7p.v6-dd.high-simple-eval-1k.binpack
  - /data/hse/S7/test80-nov2022-16tb7p-v6-dd.min.high-simple-eval-1k.binpack

  # T80 2023
  - /data/hse/S7/test80-jan2023-3of3-16tb7p-filter-v6-dd.min-mar2023.unmin.high-simple-eval-1k.binpack
  - /data/hse/S7/test80-feb2023-16tb7p-filter-v6-dd.min-mar2023.unmin.high-simple-eval-1k.binpack
  - /data/hse/S7/test80-mar2023-2tb7p.v6-sk16.min.high-simple-eval-1k.binpack
  - /data/hse/S7/test80-apr2023-2tb7p-filter-v6-sk16.min.high-simple-eval-1k.binpack
  - /data/hse/S7/test80-may2023-2tb7p.v6.min.high-simple-eval-1k.binpack
  - /data/hse/S7/test80-jun2023-2tb7p.v6-3072.min.high-simple-eval-1k.binpack
  - /data/hse/S7/test80-jul2023-2tb7p.v6-3072.min.high-simple-eval-1k.binpack
  - /data/hse/S7/test80-aug2023-2tb7p.v6.min.high-simple-eval-1k.binpack
  - /data/hse/S7/test80-sep2023-2tb7p.high-simple-eval-1k.binpack
  - /data/hse/S7/test80-oct2023-2tb7p.high-simple-eval-1k.binpack

start-from-engine-test-net: False

nnue-pytorch-branch: linrock/nnue-pytorch/L1-128
engine-test-branch: linrock/Stockfish/L1-128-nolazy
engine-base-branch: linrock/Stockfish/L1-128

num-epochs: 500
lambda: 1.0

Experiment yaml configs converted to easy_train.sh commands with:
https://github.com/linrock/nnue-tools/blob/4339954/yaml_easy_train.py

Binpacks interleaved at training time with:
official-stockfish/nnue-pytorch#259

Data filtered for high simple eval positions with:
https://github.com/linrock/nnue-data/blob/32d6a68/filter_high_simple_eval_plain.py
https://github.com/linrock/Stockfish/blob/61dbfe/src/tools/transform.cpp#L626-L655

Training data can be found at:
https://robotmoon.com/nnue-training-data/

Local elo at 25k nodes per move of
L1-128 smallnet (nnue-only eval) vs. L1-128 trained on standard S1 data:
nn-epoch399.nnue : -318.1 +/- 2.1

Passed STC:
https://tests.stockfishchess.org/tests/view/6574cb9d95ea6ba1fcd49e3b
LLR: 2.93 (-2.94,2.94) <0.00,2.00>
Total: 62432 W: 15875 L: 15521 D: 31036
Ptnml(0-2): 177, 7331, 15872, 7633, 203

Passed LTC:
https://tests.stockfishchess.org/tests/view/6575da2d4d789acf40aaac6e
LLR: 2.94 (-2.94,2.94) <0.50,2.50>
Total: 64830 W: 16118 L: 15738 D: 32974
Ptnml(0-2): 43, 7129, 17697, 7497, 49

Bench: 1397884

Co-Authored-By: mstembera

[linrock]

@mstembera i added you as a co-author to this PR and fixed the sanitizer errors

[Disservin]

So first of all nice that we have some working example of this idea.

• now from looking at the code it seems like a lot of duplication, first of all the Makefile, I mentioned one possible way to do that here Introduce dual NNUE evaluation #4910 maybe there are better ways though. The nnue code gets rather bloated by constants if conditions checking if it is a small net or big net.
  I also find the Small template bool a bit confusing
  e.g: !st->template computed<Small>()[Perspective]
  makes it look like computed is using here only the small net, while it is actually a boolean to signal small or big, I find the names big and small also not exactly ideal.
• why was the simple_eval moved into the position class?

• vondele: exposing this at the user level, i.e. 2 uci option is probably not really ideal @vondele

• additionally the caching by fishtest might not be working in the way that it should and since we would be now moving from one default net to two

[linrock]

cleaned up makefile duplication with: #4910 (comment)

cc @mstembera regarding the code style comments

[ddobbelaere]

Congrats, nice achievement!!

I find the names big and small also not exactly ideal.

Me neither. Is there anything wrong with the generic net0 (main net), net1 (aux net)? Paves the way nicely to a potential future extension.

[mstembera]

@linrock Congrats again and thanks for all your work!

@Disservin

• We could maybe implement a proper accumulator interface w/ virtual methods as a solution but it may be slower and overkill for the initial PR. I am open to better solutions.
• The big and small names can be changed to anything you propose. I just had to pick something.
• I moved simple_eval() into position because I needed to call it inside of hint_common_parent_position() and I didn't notice that evaluate_nnue.cpp includes "evaluate.h". We should be able to move it back but it only uses position stuff anyway so whatever you decide is fine by me.
• Exposing the second net in uci options can also be removed if u think it's better not there.
• Unfortunately I don't know how fishtest cashing works so I'm not of much help.

[linrock]

why was the simple_eval moved into the position class?
We should be able to move it back but it only uses position stuff anyway so whatever you decide is fine by me.

imo it's a simplification and more cohesive to move simple_eval into the position class, given it only uses position stuff.

vondele: exposing this at the user level, i.e. 2 uci option is probably not really ideal

Updated so there's only a single EvalFile UCI option for the big net.

[linrock]

The nnue code gets rather bloated by constants if conditions checking if it is a small net or big net.
I also find the Small template bool a bit confusing
e.g: !st->template computed()[Perspective]
makes it look like computed is using here only the small net, while it is actually a boolean to signal small or big, I find the names big and small also not exactly ideal.

Me neither. Is there anything wrong with the generic net0 (main net), net1 (aux net)? Paves the way nicely to a potential future extension.

We could maybe implement a proper accumulator interface w/ virtual methods as a solution but it may be slower and overkill for the initial PR. I am open to better solutions.

I agree the code could be cleaned up and easier to read, though it seems more time and discussion is needed to flush that out. Particularly if more testing needs to be done due to the risk of a slowdown from a cleaner solution.

I think of this initial PR as a proof-of-concept that dual nnue has potential, with more cleanups and improvements to come.

Another LTC passed today, tested vs. this initial PR, showing there's room for more elo here as well:
https://tests.stockfishchess.org/tests/view/65775c034d789acf40aac7e3

[Disservin]

I think of this initial PR as a proof-of-concept that dual nnue has potential, with more cleanups and improvements to come.

I'd like to avoid merging a "half" baked solution, I think fishtest is also missing some changes for this as seen by the fact that noob's workers are having problems. I'm still waiting on some feedback from @ppigazzini.

with more cleanups and improvements to come.

that is always easier said than done :D

imo it's a simplification and more cohesive to move simple_eval into the position class, given it only uses position stuff.

we have an eval namespace and I would find it weird if we decide to split different kind of evals between position and the already mentioned eval namespace. So I'd prefer it when we leave the simple_eval in the eval namespace.

trained only on positions from filtered data with high material difference

@linrock i suppose you have a repo/branch somewhere to do this? Can you link it as well?

In the meantime I could merge this into a separate branch, where other people could then can contribute to and improve this (with fishtest backed tests)? Although they could already make pr's to your branch

I'd also like to hear some feedback from @Sopel97.

[ppigazzini]

The binary seems to not declare the small net name:

./stockfish uci
Stockfish dev-20231213-4d7f9c91 by the Stockfish developers (see AUTHORS file)
id name Stockfish dev-20231213-4d7f9c91
id author the Stockfish developers (see AUTHORS file)

option name Debug Log File type string default
option name Threads type spin default 1 min 1 max 1024
option name Hash type spin default 16 min 1 max 33554432
option name Clear Hash type button
option name Ponder type check default false
option name MultiPV type spin default 1 min 1 max 500
option name Skill Level type spin default 20 min 0 max 20
option name Move Overhead type spin default 10 min 0 max 5000
option name Slow Mover type spin default 100 min 10 max 1000
option name nodestime type spin default 0 min 0 max 10000
option name UCI_Chess960 type check default false
option name UCI_AnalyseMode type check default false
option name UCI_LimitStrength type check default false
option name UCI_Elo type spin default 1320 min 1320 max 3190
option name UCI_ShowWDL type check default false
option name SyzygyPath type string default <empty>
option name SyzygyProbeDepth type spin default 1 min 1 max 100
option name Syzygy50MoveRule type check default true
option name SyzygyProbeLimit type spin default 7 min 0 max 7
option name EvalFile type string default nn-0000000000a0.nnue
uciok

This is the output of the @mstembera's PR:

./stockfish uci
Stockfish dev-20231211-ae77fdaf by the Stockfish developers (see AUTHORS file)
id name Stockfish dev-20231211-ae77fdaf
id author the Stockfish developers (see AUTHORS file)

option name Debug Log File type string default
option name Threads type spin default 1 min 1 max 1024
option name Hash type spin default 16 min 1 max 33554432
option name Clear Hash type button
option name Ponder type check default false
option name MultiPV type spin default 1 min 1 max 500
option name Skill Level type spin default 20 min 0 max 20
option name Move Overhead type spin default 10 min 0 max 5000
option name Slow Mover type spin default 100 min 10 max 1000
option name nodestime type spin default 0 min 0 max 10000
option name UCI_Chess960 type check default false
option name UCI_AnalyseMode type check default false
option name UCI_LimitStrength type check default false
option name UCI_Elo type spin default 1320 min 1320 max 3190
option name UCI_ShowWDL type check default false
option name SyzygyPath type string default <empty>
option name SyzygyProbeDepth type spin default 1 min 1 max 100
option name Syzygy50MoveRule type check default true
option name SyzygyProbeLimit type spin default 7 min 0 max 7
option name EvalFile type string default nn-0000000000a0.nnue
option name EvalFileSmall type string default nn-c01dc0ffeede.nnue
uciok

[Disservin]

We removed that since when I talked with vondele about this he’d like to not expose this as a second uci option.

[ppigazzini]

Ok, thanks, I was using the @mstembera PR to change fishtest, reverting.

[Sopel97]

Why treat small nets differently here?

[linrock]

this was a hack to disable the additional EvalFileSmall UCI option and hardcode the default smallnet name while leaving the default EvalFile option available for configuring the big net

[mstembera]

Maybe it's worth considering removing the Default net from the options also. I don't think any users can produce nets anyway and anyone who has usually just changes the name in evaluate.h Treating the nets consistently would be nice and avoid this. @vondele @Disservin What do u think?

[linrock]

the EvalFile option is currently necessary during training for local elo measurements, as it's used to load nets converted from training checkpoints without having to recompile a new binary for each one:

https://github.com/official-stockfish/nnue-pytorch/blob/master/run_games.py#L126-L129

[mstembera]

@linrock Got it. Is it then not a problem for you to not have the small file exposed for the same reason?

[linrock]

it's not a problem as long as there's at least one option to change a net. for small net test branches, i modified EvalFile to instead change the small net

[Sopel97]

This is really backwards logic.

[mstembera]

I see your point. I thought the FeatureTransformers and Networks instantiated at the top of evaluate_nnue.cpp (Lines 43-48) should match so it's clear that they are both created w/ matching dimensions. No problem if you change it.

[Sopel97]

this should loop over the list of all network types to prevent bugs in the future

[mstembera]

Not sure what you mean.

[Sopel97]

somewhere in nnue headers
constexpr auto NETWORK_TYPES[] = { Big, Small };
...


for (auto tp : NETWORK_TYPES)
    for (auto color : { WHITE, BLACK })
        st->computed(color, tp) = false

[mstembera]

I c. Feels a bit over engineered but I don't really object. I guess we would also have to add a StateInfo::computed(Color, NetSize) method. I was actually able to remove all the other StateInfo methods to access the accumulators as a result of your other suggestion.

[Sopel97]

I guess it's fine, other places are also not completely generic

[Sopel97]

this should loop over the list of all network types to prevent bugs in the future

[mstembera]

Ditto

[ppigazzini]

Why the bench of the PR is different from the bench of the version that passed the tests?

[peregrineshahin]

Why the bench of the PR is different from the bench of the version than passed the tests?

Sometimes tests passes on an older masters and they get rebased in the PR time.

[Disservin]

@mstembera where are you doing the changes? I think it be the easiest when you create a pr to linrocks branch and he merges that in. Instead of updating two pr simultaneously.

[mstembera]

@Sopel97 Thanks for the review!
@Disservin

1. I have updated my original PR [RFC] [WIP] Dual net NNUE #4898 to match this one.
2. I then moved simple_eval() back into the eval namespace
3. I then made changes on top of it for all the @Sopel97 suggestions I knew how.
   If it's ok w/ @Sopel97 it would probably be easier and more efficient if additional changes were made directly by him because he know precisely what he means.

@Disservin You are probably right but I don't know git well enough to make a PR to someone elses branch. Maybe @linrock can just grab the changes from me like I grabbed those from here w/o git?

[Disservin]

@mstembera

When creating a pr to Stockfish I guess you already do it over this interface? Here you can just click on the base repository dropdown and select linrocks fork and then you can just change the base branch to match that of this pr, which would be DualNNUE-1100-pr

[Disservin]

Result of  30 runs
==================
base (...fish-a5a76a6) =    1330268  +/- 10039
test (...fish-584d9ef) =    1474138  +/- 11768
diff                   =    +143870  +/- 12154

speedup        = +0.1082
P(speedup > 0) =  1.000

btw master is quite a bit faster after the merge of dualnet