fix: Calmar ratio sign for losing portfolios + DCA cutoff logic#1
Open
marcm0de wants to merge 1 commit into
Open
fix: Calmar ratio sign for losing portfolios + DCA cutoff logic#1marcm0de wants to merge 1 commit into
marcm0de wants to merge 1 commit into
Conversation
1. Calmar Ratio (stats.py): abs(cagr / mdd) incorrectly reports positive Calmar for losing portfolios (negative CAGR / negative MDD = positive). Changed to cagr / abs(mdd) so the sign correctly reflects portfolio direction. 2. DCA Cutoff (shadow_backtest.py): When dca_in_retirement=False, the cutoff used draw_start_date (which defaults to backtest start) via Python's 'or' short-circuit, silently killing all DCA from day one. Fixed to use retirement_date directly, which is the actual intended cutoff for DCA contributions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two logic bug fixes in the backtesting core.
1. Calmar Ratio reports positive values for losing portfolios
File:
app/core/calculations/stats.py, line 275Problem: The formula
abs(cagr / mdd)always produces a positive result. When a portfolio has negative CAGR (lost money) and negative MDD, the division yields a positive number, andabs()keeps it positive. This makes a losing portfolio look like it has a decent risk-adjusted return.Example:
abs(-5 / -30)=0.167← incorrectly positive-5 / abs(-30)=-0.167← correctly negativeFix: Changed to
cagr / abs(mdd)so the Calmar ratio preserves the sign of CAGR.2. DCA silently killed from day 1 when
dca_in_retirement=FalseFile:
app/core/shadow_backtest.py, line 597Problem: The DCA cutoff logic uses
draw_start_date or retirement_date. Sincedraw_start_dateis always set (the orchestrator defaults it to the backtest start date), Python'sorshort-circuits and always picksdraw_start_date. When a user disables DCA in retirement, their DCA contributions are stopped from the very first day of the simulation — $0 invested over the entire backtest.Fix: Changed to use
retirement_datedirectly, which is the actual intended cutoff for DCA contributions whendca_in_retirement=False.