Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 26, 2026

Thanks for assigning this issue to me. I'm starting to work on it and will keep this PR's description up to date as I form a plan and make progress.

Original prompt

This section details on the original issue you should resolve

<issue_title>[Chore] Codebase cleanup, reorganization, and dead code removal</issue_title>
<issue_description>## Summary

Comprehensive cleanup and reorganization of the MetalFish codebase to improve maintainability, reduce redundancy, and establish a cleaner architecture. This involves analyzing every file, removing unused code, merging overlapping functionality, restructuring directories, and removing all references to Stockfish and Lc0.

Current State Analysis

Directory Structure (Current)

src/
├── core/          # 7,554 lines - Chess fundamentals
├── eval/          # 5,470 lines - NNUE evaluation
├── gpu/           # 7,878 lines - GPU backends (Metal, CUDA, CPU)
├── mcts/          # 9,160 lines - MCTS implementations (MULTIPLE!)
├── search/        # 5,195 lines - Alpha-beta search
├── syzygy/        # 1,780 lines - Tablebase probing
├── uci/           # 3,591 lines - UCI protocol
├── main.cpp
├── benchmark_gpu.cpp
└── paper_benchmark.cpp

tests/
├── 19 test files with overlapping coverage
└── ~40,000 lines total

Key Issues Identified

1. Multiple MCTS Implementations (HIGH PRIORITY)

The src/mcts/ directory has overlapping implementations:

File Lines Purpose Status
thread_safe_mcts.cpp/h 2,359 Primary MCTS ✅ Keep
apple_silicon_mcts.cpp/h 1,323 Apple-specific MCTS ⚠️ Merge into thread_safe
parallel_hybrid_search.cpp/h 1,907 Hybrid AB+MCTS ⚠️ Review for merge
ab_integration.cpp/h 1,462 AB integration ⚠️ Merge with parallel_hybrid
lc0_mcts_core.h 1,077 Lc0 types (header-only) ⚠️ Review necessity
stockfish_adapter.cpp/h 693 SF compatibility ⚠️ Rename/rebrand
position_classifier.cpp/h 871 Position analysis ✅ Keep

Problem: AppleSiliconMCTSEvaluator, ThreadSafeMCTS, and ParallelHybridSearch all implement similar MCTS logic with different optimizations. This should be ONE configurable class.

2. Redundant Test Files

Multiple test files cover the same functionality:

Redundant Pair Action
test_core.cpp + test_bitboard.cpp + test_position.cpp + test_movegen.cpp Merge into test_core.cpp
test_mcts.cpp + test_mcts_module.cpp + test_mcts_comprehensive.cpp Merge into test_mcts.cpp
test_search.cpp + test_search_module.cpp + test_search_comprehensive.cpp Merge into test_search.cpp
test_gpu_nnue.cpp + test_gpu_module.cpp + test_gpu_comprehensive.cpp Merge into test_gpu.cpp
test_hybrid.cpp + test_hybrid_comprehensive.cpp Merge into test_hybrid.cpp

3. Large Files Needing Split

File Lines Recommendation
gpu_nnue_integration.cpp 2,313 Split: weights, inference, batching
search.cpp 2,191 Split: search, aspiration, pruning
uci.cpp 1,974 Split: parsing, commands, options
thread_safe_mcts.cpp 1,634 Split: tree, search, evaluation

4. Platform-Specific Code Not Properly Isolated

Files with mixed platform code that should be separated:

  • src/core/memory.cpp - Has #ifdef __APPLE__ mixed in
  • src/core/numa.h - 1,437 lines with platform-specific sections
  • src/search/apple_silicon_search.h - Should be in a platform/ directory
  • src/mcts/apple_silicon_mcts.cpp - Should be in platform/

5. Unused/Dead Code Patterns

  • src/core/shm.h and src/core/shm_linux.h - Shared memory (likely unused)
  • src/core/perft.h - Perft testing (move to tests/)
  • src/search/tune.cpp/h - Tuning infrastructure (review if used)
  • src/gpu/cpu_backend.cpp - CPU fallback (verify it's actually used)

6. Stockfish/Lc0 References That Must Be Removed (HIGH PRIORITY)

MetalFish is its own engine. All references to Stockfish and Lc0 must be removed or rebranded.

Files with Stockfish references (18 files):

  • src/gpu/gpu_accumulator_cache.h
  • src/gpu/gpu_nnue_integration.cpp
  • src/gpu/gpu_mcts_backend.h
  • src/gpu/gpu_mcts_backend.cpp
  • src/gpu/gpu_nnue_integration.h
  • src/core/numa.h
  • src/uci/engine.h
  • src/uci/benchmark.cpp
  • src/uci/uci.cpp
  • src/eval/nnue/network.cpp
  • src/mcts/thread_safe_mcts.cpp
  • src/mcts/parallel_hybrid_search.h
  • src/mcts/parallel_hybrid_search.cpp
  • src/mcts/stockfish_adapter.hRename to metalfish_adapter.h
  • src/mcts/ab_integration.h
  • src/mcts/ab_integration.cpp
  • src/mcts/stockfish_adapter.cppRename to metalfish_adapter.cpp
  • tests/test_hybrid.cpp

Files with Lc0/Leela references (8 files):

  • src/mcts/apple_silicon_mcts.h
  • src/mcts/thread_safe_mcts.h
  • src/mcts/apple_silicon_mcts.cpp
  • src/mcts/thread_safe_mcts.cpp
  • src/mcts/lc0_mcts_core.hRename to mcts_core.h
  • src/mcts/parallel_hybrid_search.cpp
  • src/mcts/stockfish_adapter.h
  • `src/mcts/stockfish_adapter...

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

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.

[Chore] Codebase cleanup, reorganization, and dead code removal

2 participants