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

Support different Net Architecture #3670

Closed
CaptainFuture1405 opened this issue Aug 22, 2021 · 2 comments
Closed

Support different Net Architecture #3670

CaptainFuture1405 opened this issue Aug 22, 2021 · 2 comments

Comments

@CaptainFuture1405
Copy link

In order to give network developers more opportunities to experiment and to support networks with different architectures, the NnueVersion should not be defined as a fixed value in nnue.c, but should be determined dynamically via the function readu_le_u32(d), see the function verify_net().
The function verify_net() itself will then be obsolete.
Depending on NnueVersion, the network parameters, e.g. kHalfDimensions, can be determined.

The easiest way to do this code extension is when the the code is prepared for a new network architecture.

@Sopel97
Copy link
Member

Sopel97 commented Aug 22, 2021

Depending on NnueVersion, the network parameters, e.g. kHalfDimensions, can be determined.

No, this is false. The NnueVersion is vague, and because "hashes" are used to identify the parts of the network we can only reliably provide validation. This could be resolved with official-stockfish/nnue-pytorch#140.

There are, however, a number of implementation issues that are not related to the way the nets are stored:

  1. One possible solution would be to make a network container providing a virtual "evaluate" method, but this introduces a virtual function call and will hinder inlining without PGO. This would also only allow using architectures from a predefined subset.
  2. The other possibility is to make the layer sizes not constants. This would prevent constant propagation optimisation, and in particular hinders loop unrolling. A lot of dispatch that is now static would be moved to the runtime.
  3. Supporting the nets of different sizes would require more optimized code paths for sizes that are not supported by the current code. A lot of code, and even more dispatch logic.
  4. Multiple feature sets would need to be supported. Some have different requirements for the maximum active feature count, and the need to account for that would prevent us from using constant sized features arrays. This would either require using a container with dynamically allocated storage, or an upper bound.
  5. It's a lot of code that won't be tested on fishtest

I don't think this is a good direction. People who experiment with different architectures can still do so by modifying the source code.

@vondele
Copy link
Member

vondele commented Feb 20, 2022

our master branch just supports one net version, by design. Users should in general just use the embedded net. Developers can change the code as needed. As architectures evolve it is quite hard to maintain compatibility.

@vondele vondele closed this as completed Feb 20, 2022
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

No branches or pull requests

3 participants