Skip to content

Remove unused code from the post NNUE accumulator merge#6898

Open
zungur wants to merge 1 commit into
official-stockfish:masterfrom
zungur:cleanups-nnue
Open

Remove unused code from the post NNUE accumulator merge#6898
zungur wants to merge 1 commit into
official-stockfish:masterfrom
zungur:cleanups-nnue

Conversation

@zungur

@zungur zungur commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

After merging the HalfKA and Threats accumulators (7c7fe32) and the subsequent removal of the double-incremental/fused update, a number of NNUE helpers and fields became unreachable. Each was verified to have zero callers/readers across the source tree:

  • FusedUpdateData logic in FullThreats: the fused-update branch of append_changed_indices and the FusedUpdateData parameter are unused; the accumulator update no longer passes fused data.
  • FullThreats::requires_refresh: never called. The live king-bucket refresh check is HalfKAv2_hm::requires_refresh (PSQFeatureSet), used in nnue_accumulator.
  • HalfKAv2_hm::append_active_indices: never called. The live active-index builder is FullThreats::append_active_indices (ThreatFeatureSet).
  • DirtyThreats::us, prevKsq and ksq: written in do_move but only read by the now-removed FullThreats::requires_refresh. Removing them also drops three stores from the do_move path.
  • Unused feature Name constants and the unused FtOneVal / HiddenMaxVal constants in nnue_common.h.
  • Two stale feature-header banner comments.

No functional change

@github-actions

Copy link
Copy Markdown

clang-format 20 needs to be run on this PR.
If you do not have clang-format installed, the maintainer will run it when merging.
For the exact version please see https://packages.ubuntu.com/plucky/clang-format-20.

(execution 27386387097 / attempt 1)

@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ca909464-4dd4-4696-aa6c-b32d0bcd17f1

📥 Commits

Reviewing files that changed from the base of the PR and between 3c15e6f and b9aa732.

📒 Files selected for processing (8)
  • src/nnue/features/full_threats.cpp
  • src/nnue/features/full_threats.h
  • src/nnue/features/half_ka_v2_hm.cpp
  • src/nnue/features/half_ka_v2_hm.h
  • src/nnue/nnue_accumulator.cpp
  • src/nnue/nnue_common.h
  • src/position.cpp
  • src/types.h
💤 Files with no reviewable changes (4)
  • src/types.h
  • src/nnue/nnue_common.h
  • src/position.cpp
  • src/nnue/features/full_threats.cpp
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/nnue/nnue_accumulator.cpp
  • src/nnue/features/half_ka_v2_hm.h
  • src/nnue/features/full_threats.h

📝 Walkthrough

Walkthrough

This PR refactors NNUE threat feature handling to remove fused-update infrastructure and simplify state tracking. The DirtyThreats struct is reduced to contain only a list field, removing caller-supplied metadata. FullThreats removes its FusedUpdateData helper type and simplifies the append_changed_indices signature by removing fusedData and first parameters. HalfKAv2_hm removes active-index scanning and aligns its API accordingly. Call sites in nnue_accumulator are updated to use the simplified signature. Unused constants FtOneVal and HiddenMaxVal are removed.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

After merging the HalfKA and Threats accumulators (7c7fe32) and the
subsequent removal of the double-incremental/fused update, a number of
NNUE helpers and fields became unreachable. Each was verified to have
zero callers/readers across the source tree:

- FusedUpdateData logic in FullThreats: the fused-update branch of
  append_changed_indices and the FusedUpdateData parameter are unused;
  the accumulator update no longer passes fused data.
- FullThreats::requires_refresh: never called. The live king-bucket
  refresh check is HalfKAv2_hm::requires_refresh (PSQFeatureSet), used
  in nnue_accumulator.
- HalfKAv2_hm::append_active_indices: never called. The live active-index
  builder is FullThreats::append_active_indices (ThreatFeatureSet).
- DirtyThreats::us, prevKsq and ksq: written in do_move but only read by
  the now-removed FullThreats::requires_refresh. Removing them also drops
  three stores from the do_move path.
- Unused feature Name constants and the unused FtOneVal / HiddenMaxVal
  constants in nnue_common.h.
- Two stale feature-header banner comments.

No functional change
@github-actions

Copy link
Copy Markdown

clang-format 20 needs to be run on this PR.
If you do not have clang-format installed, the maintainer will run it when merging.
For the exact version please see https://packages.ubuntu.com/plucky/clang-format-20.

(execution 27387019374 / attempt 1)

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.

1 participant