gitadmin/foxhunt
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

cleanup: delete 92 scattered .md files and .serena artifacts

Browse Source 
Remove stale test reports, quick-start guides, benchmark analyses,
profiling reports, and tool artifacts from across the workspace.
Keeps only root README.md per crate/service.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
jgrusewski
2026-03-01 22:42:36 +01:00
parent 5296b44fe7
commit 4a1add5806
78 changed files with 0 additions and 29377 deletions
Download Patch File Download Diff File Expand all files Collapse all files

286
crates/ml/docs/codebase-cleanup/DQN_PARAMETER_CONSISTENCY_AUDIT.md
View File

@@ -1,286 +0,0 @@
# DQN Parameter Consistency Audit Report
**Date**: 2025-11-27
**Auditor**: Code Quality Analyzer
**Scope**: Rainbow DQN Feature Flags Consistency Check
---
## Executive Summary
**CRITICAL FINDING**: `use_double_dqn` is **MISSING** from the hyperopt search space (`DQNParams`) but **HARDCODED** to `true` in the conversion function. This violates SINGLE SOURCE OF TRUTH and prevents hyperopt from tuning this parameter.
### Consistency Matrix
| Parameter | DQNParams (Hyperopt) | DQNHyperparameters (Trainer) | In Search Space? | Hardcoded in Conversion? | Should Be Fixed? |
|-----------|---------------------|------------------------------|------------------|--------------------------|------------------|
| `use_double_dqn` | ❌ **MISSING** | `true` (line 569) | ❌ **NO** | ✅ **YES** (line 1981) | ✅ **CRITICAL** |
| `use_dueling` | `true` (line 336) | `true` (line 631) | ✅ YES | ❌ NO | ❌ NO |
| `use_per` | `true` (line 333) | `true` (line 626) | ✅ YES | ❌ NO | ❌ NO |
| `use_noisy_nets` | `true` (line 359) | `true` (line 644) | ✅ YES | ❌ NO | ❌ NO |
| `use_distributional` | `false` (line 355) | `true` (line 638) | ✅ YES | ❌ NO | ⚠️ **INCONSISTENT** |
---
## Detailed Findings
### 1. **CRITICAL BUG**: `use_double_dqn` Missing from Search Space
**Location**: `/home/jgrusewski/Work/foxhunt/ml/src/hyperopt/adapters/dqn.rs`
**Problem**:
```rust
// DQNParams struct (lines 160-319)
pub struct DQNParams {
// ... 40+ parameters ...
pub use_per: bool, // ✅ Present
pub use_dueling: bool, // ✅ Present
pub use_distributional: bool, // ✅ Present
pub use_noisy_nets: bool, // ✅ Present
// ❌ use_double_dqn: MISSING!
}
// Hardcoded in conversion (line 1981)
use_double_dqn: true, // Production feature: --use-double-dqn
```
**Impact**:
- Hyperopt **CANNOT** tune this parameter
- All hyperopt trials use the same value (`true`) regardless of search strategy
- Potential performance gains from ablation study (testing `false` vs `true`) are lost
- Violates SINGLE SOURCE OF TRUTH principle
**Recommendation**: Add `use_double_dqn` to `DQNParams` struct with default `true`
---
### 2. ⚠️ **INCONSISTENCY**: `use_distributional` Defaults Mismatch
**DQNParams Default** (line 355):
```rust
use_distributional: false, // DISABLED until BUG #36 fixed (was: true)
```
**DQNHyperparameters Default** (line 638):
```rust
use_distributional: true, // Default: enabled (C51 distributional RL)
```
**Root Cause**: BUG #36 (Candle scatter_add gradient flow issue)
**Comment Explains Context** (lines 341-355):
```rust
// =============================================================================
// WAVE 23 P1 FIX: C51 DISTRIBUTIONAL RL DISABLED (BUG #36)
// =============================================================================
// BUG #36: Candle's scatter_add has broken gradient flow in backward pass
// Symptom: 40% of trials experience complete gradient collapse at Epoch 2
// Root Cause: CPU scatter loop breaks autograd graph in project_distribution()
// Status: BLOCKED by external library bug
// Re-enable: After Candle library fixes scatter_add or we implement workaround
//
// Evidence: /tmp/WAVE23_CAMPAIGN_FINAL_ANALYSIS.md
// - 60% success rate WITH C51 enabled
// - Expected 95%+ success rate WITH C51 disabled (standard DQN proven stable)
//
// Performance: Standard DQN achieves Sharpe 0.77-2.0 WITHOUT distributional RL
// =============================================================================
```
**Impact**:
- **Hyperopt trials** disable distributional RL (safe)
- **Production trainer defaults** enable distributional RL (risky if using defaults)
- **Documented workaround**: BUG #36 is known and intentionally disabled in hyperopt
- **Test coverage**: 60% success rate with C51 enabled → intentional disable
**Recommendation**:
- **Option A (Conservative)**: Change `DQNHyperparameters` default to `false` to match hyperopt
- **Option B (Documented)**: Keep as-is but add prominent comment warning about BUG #36
- **Option C (Fix root cause)**: Implement Candle scatter_add workaround or wait for upstream fix
---
### 3. ✅ **CONSISTENT**: `use_dueling`, `use_per`, `use_noisy_nets`
All three parameters are **correctly defined** in both structs with **matching defaults**:
| Parameter | DQNParams | DQNHyperparameters | Status |
|-----------|-----------|-------------------|--------|
| `use_dueling` | `true` (line 336) | `true` (line 631) | ✅ CONSISTENT |
| `use_per` | `true` (line 333) | `true` (line 626) | ✅ CONSISTENT |
| `use_noisy_nets` | `true` (line 359) | `true` (line 644) | ✅ CONSISTENT |
**Notes**:
- All three are **tunable** in hyperopt search space
- All three **default to enabled** (Rainbow DQN standard configuration)
- Comments explain Rainbow DQN rationale (lines 143-146, 333-359)
---
## Root Cause Analysis
### Why `use_double_dqn` is Missing
**Hypothesis 1: Legacy Refactoring**
- Double DQN was likely added **before** hyperopt integration
- When hyperopt search space was defined, `use_double_dqn` was already production-standard
- Developers assumed it should **always be enabled**, so excluded it from tuning
**Hypothesis 2: Performance Certainty**
- Double DQN is a well-established improvement over vanilla DQN (prevents Q-value overestimation)
- Literature consensus: Double DQN should **always** be enabled
- Unlike other Rainbow components (C51, Noisy Nets), Double DQN has **no known downsides**
**Evidence**:
- Comment at line 1981: `// Production feature: --use-double-dqn`
- Implies it's a **production standard**, not a tunable hyperparameter
- Other Rainbow flags have **explicit tuning rationale** in comments
---
## Recommended Action Plan
### Priority 1: Add `use_double_dqn` to Search Space
**File**: `/home/jgrusewski/Work/foxhunt/ml/src/hyperopt/adapters/dqn.rs`
**Changes Required**:
1. **Add field to `DQNParams` struct** (after line 240):
```rust
/// Enable Double DQN to reduce Q-value overestimation bias
/// Default: true (Rainbow DQN standard, production-validated)
/// Expected impact: +5-10% stability, prevents overestimation
pub use_double_dqn: bool,
```
2. **Add default value** (after line 359):
```rust
use_double_dqn: true, // Wave 2.0: Default ENABLED (production standard)
```
3. **Remove hardcoded value** (line 1981):
```rust
// BEFORE:
use_double_dqn: true, // Production feature: --use-double-dqn
// AFTER:
use_double_dqn: params.use_double_dqn, // Tunable boolean (default: true)
```
4. **Update search space bounds** (if needed):
```rust
// In search space definition
use_double_dqn: [0.0, 1.0], // Boolean: 0.0=false, 1.0=true, threshold at 0.5
```
### Priority 2: Resolve `use_distributional` Inconsistency
**Option A (Recommended)**: Align `DQNHyperparameters` default with hyperopt
**File**: `/home/jgrusewski/Work/foxhunt/ml/src/trainers/dqn/config.rs`
**Change** (line 638):
```rust
// BEFORE:
use_distributional: true, // Default: enabled (C51 distributional RL)
// AFTER:
use_distributional: false, // WAVE 23: DISABLED until BUG #36 fixed (see hyperopt/adapters/dqn.rs:341-355)
```
**Rationale**:
- Prevents accidental use of buggy C51 implementation
- Aligns production defaults with hyperopt (SINGLE SOURCE OF TRUTH)
- 60% success rate is unacceptable for production
- Comment explains **why** it's disabled and **when** to re-enable
---
## Testing Strategy
### Test 1: Verify Search Space Completeness
```bash
# After adding use_double_dqn to DQNParams
cargo test --package ml --lib hyperopt::adapters::dqn::tests::test_default_dqn_params -- --exact
```
**Expected**: Test passes, `use_double_dqn` defaults to `true`
### Test 2: Verify Conversion Function
```bash
# Ensure hyperparams use params.use_double_dqn instead of hardcoded true
cargo test --package ml --lib hyperopt::adapters::dqn::tests::test_dqn_params_to_hyperparameters -- --exact
```
**Expected**: Test passes, `use_double_dqn` flows from `DQNParams` → `DQNHyperparameters`
### Test 3: Verify Production Defaults Alignment
```bash
# Compare DQNParams::default() with DQNHyperparameters::default()
cargo test --package ml --lib trainers::dqn::config::tests::test_default_alignment -- --exact
```
**Expected**: All Rainbow flags have matching defaults
---
## Appendix: Full Parameter Inventory
### Rainbow DQN Feature Flags (All Locations)
| File | Struct | Line | Field | Default | Purpose |
|------|--------|------|-------|---------|---------|
| `hyperopt/adapters/dqn.rs` | `DQNParams` | 188 | `use_per` | `true` | Prioritized Experience Replay |
| `hyperopt/adapters/dqn.rs` | `DQNParams` | 203 | `use_dueling` | `true` | Dueling DQN Architecture |
| `hyperopt/adapters/dqn.rs` | `DQNParams` | 222 | `use_distributional` | `false` | C51 Distributional RL (BUG #36) |
| `hyperopt/adapters/dqn.rs` | `DQNParams` | 240 | `use_noisy_nets` | `true` | Noisy Networks Exploration |
| `hyperopt/adapters/dqn.rs` | `DQNParams` | **MISSING** | `use_double_dqn` | **N/A** | **CRITICAL BUG** |
| `trainers/dqn/config.rs` | `DQNHyperparameters` | 303 | `use_double_dqn` | `true` | Double DQN (line 569) |
| `trainers/dqn/config.rs` | `DQNHyperparameters` | 399 | `use_per` | `true` | Prioritized Experience Replay (line 626) |
| `trainers/dqn/config.rs` | `DQNHyperparameters` | 405 | `use_dueling` | `true` | Dueling DQN Architecture (line 631) |
| `trainers/dqn/config.rs` | `DQNHyperparameters` | 416 | `use_distributional` | `true` | C51 Distributional RL (line 638) |
| `trainers/dqn/config.rs` | `DQNHyperparameters` | 427 | `use_noisy_nets` | `true` | Noisy Networks Exploration (line 644) |
---
## Compliance Checklist
- [x] Identified all Rainbow DQN feature flags
- [x] Compared defaults between `DQNParams` and `DQNHyperparameters`
- [x] Flagged `use_double_dqn` as **MISSING** from search space
- [x] Flagged `use_distributional` as **INCONSISTENT** (false vs true)
- [x] Documented BUG #36 context for `use_distributional`
- [x] Provided actionable fix recommendations
- [x] Created testing strategy for validation
- [x] Generated consistency matrix for stakeholder review
---
## Conclusion
**SINGLE SOURCE OF TRUTH VIOLATION DETECTED**
1. **CRITICAL**: `use_double_dqn` must be added to `DQNParams` struct
2. **INCONSISTENT**: `use_distributional` defaults differ due to BUG #36 (documented)
3. **RECOMMENDATION**: Fix Priority 1 immediately, evaluate Priority 2 based on BUG #36 resolution timeline
**Risk Assessment**:
- **Current State**: Hyperopt cannot explore Double DQN ablation
- **Impact**: Potential 5-10% performance gain lost
- **Mitigation**: Add parameter to search space, re-run hyperopt trials
**Next Steps**:
1. Add `use_double_dqn` to `DQNParams` (10 min fix)
2. Update conversion function to use `params.use_double_dqn` (5 min fix)
3. Run test suite to verify consistency (15 min validation)
4. Consider aligning `use_distributional` defaults (defer until BUG #36 resolved)
---
**Report Generated**: 2025-11-27
**Audit Tool**: Claude Code (Code Quality Analyzer)
**Codebase**: Foxhunt ML Trading System