-
Notifications
You must be signed in to change notification settings - Fork 1
[Chore] Comprehensive documentation and comment cleanup #26
Description
Summary
Create professional, comprehensive documentation for MetalFish and clean up all comments throughout the codebase. Remove redundant, obvious, or unprofessional comments while adding meaningful documentation that helps developers understand the code.
Current State
Documentation Gaps
- Only 1 file has Doxygen-style documentation (
@brief,@param,@return) - Most header files lack API documentation
- No architecture overview document
- No contribution guidelines
- Comment density varies wildly (6% to 27% across files)
Comment Issues Found
1. Empty or Placeholder Comments
// Found in multiple files:
//Files affected: gpu_nnue_integration.cpp, nnue_weight_accessor.h, numa.h, types.h
2. Comments Stating the Obvious
// Check if buffer is valid
bool is_valid() const;
// Initialize with network dimensions
void init(int dims);
// Returns true if successful
bool load();These add no value and should be removed or replaced with meaningful documentation.
3. Commented-Out Code
Found in: memory.cpp, position.cpp, search.cpp, tbprobe.cpp
All commented-out code should be removed - we have git history.
4. Informal/Unprofessional Comments
Comments like:
- "// hack to make this work"
- "// this is ugly but..."
- "// TODO: fix later"
- "// magic number"
5. Attribution Comments That Need Updating
Per issue #18, all references to Stockfish/Lc0 in comments need to be removed or reworded.
Requirements
1. File-Level Documentation
Every source file should have a documentation block after the copyright header:
/*
MetalFish - A GPU-accelerated UCI chess engine
Copyright (C) 2025 Nripesh Niketan
Licensed under GPL-3.0
*/
/**
* @file bitboard.cpp
* @brief Bitboard manipulation and attack generation utilities
*
* This file implements efficient bitboard operations for chess move
* generation, including sliding piece attacks using magic bitboards
* and pre-computed attack tables.
*/2. Class/Struct Documentation
All public classes should have Doxygen documentation:
/**
* @class ThreadSafeMCTS
* @brief Thread-safe Monte Carlo Tree Search implementation
*
* Implements MCTS with virtual loss for parallel search across
* multiple threads. Optimized for Apple Silicon with unified
* memory architecture.
*
* @note Thread-safe for concurrent access during search
* @see MCTSNode, MCTSConfig
*/
class ThreadSafeMCTS {3. Function Documentation
All public functions should have parameter and return documentation:
/**
* @brief Performs MCTS search from the given position
*
* @param pos The chess position to search from
* @param limits Search constraints (time, nodes, depth)
* @return The best move found by the search
*
* @throws std::runtime_error if position is invalid
*/
Move search(const Position& pos, const SearchLimits& limits);4. Inline Comments
DO include comments for:
- Complex algorithms (explain the "why", not the "what")
- Non-obvious optimizations
- Important invariants
- Magic numbers (with explanation)
- Platform-specific code
DO NOT include comments for:
- Self-explanatory code
- Obvious operations (
// increment counter) - Restating the code in English
- TODO/FIXME without ticket references
Example of good inline comments:
// Use SIMD reduction for Apple Silicon (2x faster than scalar loop)
#ifdef __ARM_NEON
sum = vaddvq_s32(accumulator);
#else
for (int i = 0; i < 4; i++) sum += accumulator[i];
#endif
// Early exit: positions with >6 pieces never hit tablebase
if (popcount(pos.pieces()) > 6) return SCORE_NONE;Detailed Tasks
Phase 1: Remove Bad Comments
1.1 Remove Empty Comments
-
src/gpu/gpu_nnue_integration.cpp:161- empty// -
src/gpu/nnue_weight_accessor.h:112- empty// -
src/core/numa.h- multiple empty comments (lines 433, 435, 551, 559, 565, 855, 860) -
src/core/types.h:13- empty//
1.2 Remove Obvious Comments
Replace or remove comments like:
-
// Check if buffer is valid→ Remove (method name is clear) -
// Initialize with network dimensions→ Remove -
// Returns true if successful→ Use@returnin function doc instead -
// Constructor/// Destructor→ Remove entirely
1.3 Remove Commented-Out Code
- Audit all files for
#if 0blocks - Remove any commented-out function implementations
- Remove any
// old code:sections
1.4 Clean Up TODO/FIXME
- Remove TODOs that are done
- Convert important TODOs to GitHub issues
- Remove vague TODOs without actionable items
Phase 2: Add File Documentation
Add documentation headers to all files:
Core Module (src/core/)
-
bitboard.cpp/h- Bitboard operations -
movegen.cpp/h- Move generation -
position.cpp/h- Position representation -
types.h- Type definitions -
memory.cpp/h- Memory management -
misc.cpp/h- Utility functions -
numa.h- NUMA support
Search Module (src/search/)
-
search.cpp/h- Alpha-beta search -
movepick.cpp/h- Move ordering -
thread.cpp/h- Thread management -
timeman.cpp/h- Time management -
tt.cpp/h- Transposition table -
history.h- History heuristics -
tune.cpp/h- Parameter tuning
MCTS Module (src/mcts/)
-
thread_safe_mcts.cpp/h- Main MCTS implementation -
apple_silicon_mcts.cpp/h- Apple Silicon optimizations -
parallel_hybrid_search.cpp/h- Hybrid search -
ab_integration.cpp/h- AB integration -
position_classifier.cpp/h- Position analysis -
mcts_core.h- Core MCTS types
GPU Module (src/gpu/)
-
backend.h- GPU backend interface -
gpu_nnue_integration.cpp/h- GPU NNUE -
gpu_mcts_backend.cpp/h- GPU MCTS backend -
gpu_accumulator_cache.cpp/h- Accumulator caching -
metal/metal_backend.mm- Metal implementation -
cuda/cuda_backend.cu- CUDA implementation
Eval Module (src/eval/)
-
evaluate.cpp/h- Evaluation interface -
score.cpp/h- Score utilities -
nnue/*.cpp/h- NNUE implementation files
UCI Module (src/uci/)
-
uci.cpp/h- UCI protocol -
engine.cpp/h- Engine coordination -
benchmark.cpp/h- Benchmarking -
ucioption.cpp/h- UCI options
Phase 3: Add API Documentation
3.1 Public Classes
Add Doxygen documentation to all public classes:
-
Position -
Move -
ThreadSafeMCTS -
GPUNNUEManager -
Backend -
TranspositionTable -
ThreadPool -
TimeManagement
3.2 Public Functions
Document all public API functions with:
- Brief description
- Parameter descriptions
- Return value description
- Exception specifications (if any)
- Usage examples (for complex APIs)
Phase 4: Create Documentation Files
4.1 Architecture Documentation
Create docs/ARCHITECTURE.md:
- High-level system overview
- Module descriptions and responsibilities
- Data flow diagrams
- Threading model
4.2 API Reference
Create docs/API.md:
- Public API documentation
- UCI command reference
- Configuration options
4.3 Development Guide
Create docs/DEVELOPMENT.md:
- Build instructions
- Testing guide
- Code style guide
- Contribution workflow
4.4 Update README.md
- Add badges (build status, license)
- Improve installation instructions
- Add usage examples
- Link to documentation
Phase 5: Comment Style Consistency
5.1 Standardize Comment Style
- Use
//for single-line comments - Use
/** */for documentation blocks - Use
/* */for multi-line explanatory comments - Consistent capitalization and punctuation
5.2 Comment Formatting
// Good: Single line comment with proper spacing
int value = compute(); // Inline comment after code
/**
* Good: Multi-line documentation
* with proper formatting.
*/
/* Good: Multi-line explanatory comment
for complex algorithm explanation */Files Requiring Most Attention
Based on analysis, these files need the most work:
| File | Issues |
|---|---|
src/core/numa.h |
7+ empty comments, no docs |
src/gpu/gpu_nnue_integration.cpp |
Empty comments, dense code |
src/search/search.cpp |
414 comments, many redundant |
src/mcts/thread_safe_mcts.cpp |
Complex code, minimal docs |
src/gpu/backend.h |
Obvious comments to remove |
Documentation Generation
Doxygen Setup
# Install Doxygen
brew install doxygen graphviz
# Generate documentation
cd metalfish
doxygen DoxyfileCreate Doxyfile with:
- Project name: MetalFish
- Output:
docs/api/ - Include diagrams
- Extract all documentation
Acceptance Criteria
- All source files have file-level documentation
- All public classes have Doxygen documentation
- All public functions have parameter/return docs
- No empty comments remain
- No obvious/redundant comments remain
- No commented-out code remains
- No TODO/FIXME without issue references
-
docs/ARCHITECTURE.mdexists -
docs/DEVELOPMENT.mdexists - README.md is comprehensive and professional
- Doxygen generates clean documentation
- Code compiles without documentation warnings
Priority
Medium - Good documentation is essential for maintainability and onboarding new contributors. Should be done after core functionality is stable.