Skip to content

fix(backtesting): annualize bar-count equity curves by trading days (#12)#37

Open
bradsmithmba wants to merge 1 commit into
cloudtrainerwork:masterfrom
bradsmithmba:fix/annualized-return-trading-days
Open

fix(backtesting): annualize bar-count equity curves by trading days (#12)#37
bradsmithmba wants to merge 1 commit into
cloudtrainerwork:masterfrom
bradsmithmba:fix/annualized-return-trading-days

Conversation

@bradsmithmba

Copy link
Copy Markdown

Summary

PerformanceCalculator.calculate_all_metrics() annualized returns by dividing the period by 365.25 in both branches:

if 'datetime' in equity_curve.columns:
    days = (last - first).days        # calendar days
else:
    days = len(equity_curve)          # trading-bar count
years = max(days / 365.25, 1/self.trading_days)
  • With a datetime column, days is elapsed calendar days, so /365.25 is correct.
  • Without one, days is a count of trading bars, and dividing that by 365.25 over-annualizes the return.

Example: 252 trading bars at +20% total reported ~30% annualized instead of ~20%.

Closes #12.

Fix

Annualize the no-datetime branch by self.trading_days; leave the calendar branch unchanged.

if 'datetime' in equity_curve.columns:
    days = (last - first).days
    years = max(days / 365.25, 1 / self.trading_days)
else:
    bars = len(equity_curve)
    years = max(bars / self.trading_days, 1 / self.trading_days)

trading_days was already used by the Sharpe/Sortino/volatility calculations; this makes annualized_return consistent with it for bar-indexed curves. (The issue's "trading_days is never used" note was slightly off — it was used everywhere except this annualization.)

Verification

252 bars, +20% total -> annualized_return = 0.20   (was ~0.30)

Adds test_annualized_return_without_datetime_uses_trading_days.

Scope note

tests/backtesting/test_performance.py has 3 pre-existing failures unrelated to this change — test_win_rate_calculation, test_sharpe_ratio_calculation, and test_annualization all assert incorrect expected values (wrong oracles). They use the calendar branch this PR does not touch and are tracked by #14. My new test passes; those remain for #14.

🤖 Generated with Claude Code

annualized_return divided the period by 365.25 in both branches of
calculate_all_metrics. That is correct when the equity curve has a datetime
column (elapsed calendar days / calendar days per year), but wrong when it
does not: there each row is a trading bar, and a trading-bar count divided
by 365.25 over-annualizes the return (e.g. 252 bars at +20% reported ~30%
instead of ~20%).

Use self.trading_days to annualize the no-datetime (bar-count) branch; leave
the calendar-days branch unchanged. trading_days was already honored by the
Sharpe/Sortino/volatility calculations — this aligns annualized_return with
it for bar-indexed curves.

Verified: 252 bars at +20% total now yields 0.20 annualized (was ~0.30).
Adds a regression test for the bar-count branch.

Closes cloudtrainerwork#12

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

annualized_return ignores trading_days constructor parameter, hardcodes 365.25

1 participant