Conversation
Jaco-Ren
marked this pull request as ready for review
August 12, 2026 16:17
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
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.
Fixes #606
Summary
Root cause and impact
The trades-based Monte Carlo scenarios used cumulative total return divided by maximum drawdown, while the
originalcolumn used CAGR divided by maximum drawdown. On multi-year backtests, this put adjacent values on different scales, inflated scenario Calmar ratios, and made the resulting p-value misleading.For example, a curve growing from 100 to 121 over two years with a 10% maximum drawdown now produces a 10% annualized return and a Calmar ratio of 1, rather than using the 21% cumulative return and reporting 2.1.
Coordination with #608
This change is intentionally separate from #608, which addresses #607 by changing how shuffled trade equity paths are reconstructed. This PR only calculates metrics from an already reconstructed curve and does not change trade ordering, compounding, drawdown, volatility, or Sharpe calculations.
Both changes touch
monte_carlo_trades.py, so a small rebase may be required if #608 merges first, but the fixes are logically independent.Tests
The full Jesse suite was not run locally because the complete Jesse/Ray dependency environment is unavailable. Repository CI will run the supported platform and Python matrix.