Skip to content

[Chore] Comprehensive documentation and comment cleanup #26

@NripeshN

Description

@NripeshN

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 @return in function doc instead
  • // Constructor / // Destructor → Remove entirely

1.3 Remove Commented-Out Code

  • Audit all files for #if 0 blocks
  • 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 Doxyfile

Create 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.md exists
  • docs/DEVELOPMENT.md exists
  • 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.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions