Skip to content

Commit 0f1c1ca

Browse files
committed
fix: [Bug] maxDrawdown uses opposite sign conventions in report.t (#1044)
1 parent 55b4b97 commit 0f1c1ca

3 files changed

Lines changed: 30 additions & 28 deletions

File tree

‎packages/strategies/src/report.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,8 @@ export function mergeReports(
508508
// continuation's volatility / sharpeProxy, since the continuation reflects
509509
// the most recent regime. Callers that need a mathematically combined
510510
// metric should recompute from raw timeseries; this merge is best-effort.
511+
// `maxDrawdown` is a non-negative ratio (0 = no drawdown, 1 = total loss),
512+
// so the larger value is the more severe drawdown.
511513
const maxDrawdown =
512514
base.riskMetrics.maxDrawdown.compareTo(
513515
continuation.riskMetrics.maxDrawdown

‎packages/strategies/src/risk-metrics.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,10 @@ describe("valueAtRisk", () => {
153153
});
154154

155155
describe("maxDrawdown", () => {
156-
it("returns the most negative entry", () => {
156+
it("returns the largest decline as a non-negative fraction", () => {
157157
const worst = maxDrawdown(series("0", "-0.02", "-0.05", "-0.01"));
158158

159-
expect(worst.toString()).toBe("-0.050000000000000000");
159+
expect(worst.toString()).toBe("0.050000000000000000");
160160
});
161161

162162
it("returns zero for an empty series", () => {
@@ -171,7 +171,7 @@ describe("maxDrawdown", () => {
171171

172172
it("returns the only entry of a single-element series", () => {
173173
expect(maxDrawdown(series("-0.03")).toString()).toBe(
174-
"-0.030000000000000000"
174+
"0.030000000000000000"
175175
);
176176
});
177177
});
@@ -186,7 +186,7 @@ describe("computeRiskMetrics", () => {
186186
confidenceLevel: 0.95,
187187
});
188188

189-
expect(metrics.maxDrawdown.toString()).toBe("-0.050000000000000000");
189+
expect(metrics.maxDrawdown.toString()).toBe("0.050000000000000000");
190190
expect(metrics.sharpeRatio.toString()).toBe("-0.200000000000000000");
191191
expect(metrics.valueAtRisk.toString()).toBe("0.050000000000000000");
192192
});

‎packages/strategies/src/risk-metrics.ts‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,27 +7,27 @@ import { Decimal } from "./decimal";
77
*
88
* - Arithmetic runs at scale 18, or the widest input scale when that is
99
* larger, so no input is rounded before an operation.
10-
* - `riskFreePerPeriod` is a per-period rate subtracted from the mean return.
10+
* - "riskFreePerPeriod" is a per-period rate subtracted from the mean return.
1111
* A caller holding an annual rate converts it first.
1212
* - Dispersion uses the population (n) second central moment, the usual
13-
* backtest convention for a fixed sample. `varianceMode: "sample"` switches
13+
* backtest convention for a fixed sample. "varianceMode: "sample"" switches
1414
* to the unbiased (n - 1) denominator, which divides the ratio by
15-
* `sqrt(n / (n - 1))`.
16-
* - `sharpeRatio` returns the per-period ratio. `annualizeSharpe` scales it by
17-
* `sqrt(periodsPerYear)`, the convention for i.i.d. periods. Use 365 for a
15+
* "sqrt(n / (n - 1))".
16+
* - "sharpeRatio" returns the per-period ratio. "annualizeSharpe" scales it by
17+
* "sqrt(periodsPerYear)", the convention for i.i.d. periods. Use 365 for a
1818
* daily stablecoin-vault series, 52 weekly, 12 monthly.
19-
* - `maxDrawdown` is a signed decline, so it is 0 or lower. `valueAtRisk` is a
20-
* loss magnitude, so a positive result is a loss. The two signs differ by
21-
* intent and each is documented on its own function.
19+
* - "maxDrawdown" is a non-negative magnitude, so it is 0 or higher.
20+
* "valueAtRisk" is also a loss magnitude, so a positive result is a loss.
21+
* Both are quoted as positive losses, matching the report field convention.
2222
*/
2323

2424
/** Scale used for ratio arithmetic. Stroops are too coarse for a variance. */
2525
const METRICS_SCALE = 18;
2626

2727
export interface RiskMetrics {
28-
/** Most negative entry of the drawdown series, 0 or lower. */
28+
/** Most negative entry of the drawdown series as a magnitude, 0 or higher. */
2929
readonly maxDrawdown: Decimal;
30-
/** Per-period Sharpe ratio. Annualize with `annualizeSharpe`. */
30+
/** Per-period Sharpe ratio. Annualize with "annualizeSharpe". */
3131
readonly sharpeRatio: Decimal;
3232
/** Loss magnitude at the requested confidence; negative means a gain. */
3333
readonly valueAtRisk: Decimal;
@@ -36,7 +36,7 @@ export interface RiskMetrics {
3636
export type VarianceMode = "population" | "sample";
3737

3838
/**
39-
* `(mean(returns) - riskFreePerPeriod) / stdev(returns)`, computed per period.
39+
* "(mean(returns) - riskFreePerPeriod) / stdev(returns)", computed per period.
4040
*
4141
* Throws for fewer than two returns, since one observation has no dispersion
4242
* to divide by, and for an all-identical series, which leaves a zero
@@ -78,7 +78,7 @@ export function sharpeRatio(
7878
return excessMean.div(decimalSqrt(variance));
7979
}
8080

81-
/** Scales a per-period Sharpe ratio to a yearly one by `sqrt(periodsPerYear)`. */
81+
/** Scales a per-period Sharpe ratio to a yearly one by "sqrt(periodsPerYear)". */
8282
export function annualizeSharpe(
8383
perPeriod: Decimal,
8484
periodsPerYear: Decimal | number
@@ -94,19 +94,18 @@ export function annualizeSharpe(
9494

9595
/**
9696
* Historical value at risk: the k-th smallest return, with
97-
* `k = ceil((1 - confidenceLevel) * returns.length)` clamped into `[1, n]`.
97+
* "k = ceil((1 - confidenceLevel) * returns.length)" clamped into "[1, n]".
9898
*
9999
* The quantile is negated, so a positive result is the loss the
100-
* `(1 - confidenceLevel)` lower tail reaches, and a negative result means even
101-
* that tail stayed above break-even.
100+
* "(1 - confidenceLevel)" lower tail reaches, and a negative result means even
101+
* that tail stayed above break-ven.
102102
*
103-
* Throws for a confidence outside the open interval `(0, 1)`, and for an empty
103+
* Throws for a confidence outside the open interval "(0, 1)", and for an empty
104104
* series, which has no order statistics.
105105
*/
106106
export function valueAtRisk(
107107
returns: readonly Decimal[],
108-
confidenceLevel: Decimal | number
109-
): Decimal {
108+
confidenceLevel: Decimal | number): Decimal {
110109
const confidence = toDecimalInput(confidenceLevel);
111110
if (!confidence.isPositive() || !confidence.lt(Decimal.one())) {
112111
throw new RangeError("confidenceLevel must be strictly between 0 and 1");
@@ -127,15 +126,16 @@ export function valueAtRisk(
127126

128127
const threshold = sorted[Number(clamped) - 1];
129128
if (threshold === undefined) {
130-
throw new RangeError("valueAtRisk rank fell outside the series");
129+
throw new RangeError("valueAtRIsk rank fell outside the series");
131130
}
132131
return threshold.neg();
133132
}
134133

135134
/**
136135
* Most negative entry of the drawdown series, where each entry is the running
137-
* `value / peak - 1` decline. An empty series, and a series that never leaves
138-
* its running peak, both give `0`.
136+
* "value / peak - 1" decline. The result is reported as a non-negative
137+
* magnitude: the sign of the worst decline is flipped, so a series that never
138+
* leaves its running peak gives "0". An empty series also gives "0".
139139
*/
140140
export function maxDrawdown(drawdown: readonly Decimal[]): Decimal {
141141
const scale = workingScale(drawdown);
@@ -145,7 +145,7 @@ export function maxDrawdown(drawdown: readonly Decimal[]): Decimal {
145145
worst = value.rescale(scale);
146146
}
147147
}
148-
return worst;
148+
return worst.neg();
149149
}
150150

151151
/** Computes all three metrics from the collector's series in one call. */
@@ -165,7 +165,7 @@ export function computeRiskMetrics(
165165
options.riskFreePerPeriod ?? Decimal.zero(),
166166
options.varianceMode ?? "population"
167167
),
168-
valueAtRisk: valueAtRisk(returns, options.confidenceLevel ?? 0.95),
168+
valueAtRisk: valueAtRIsk(returns, options.confidenceLevel ?? 0.95),
169169
};
170170
}
171171

@@ -226,7 +226,7 @@ function integerSqrt(value: bigint): bigint {
226226
}
227227

228228
/**
229-
* Smallest integer greater than or equal to `value`. Truncation toward zero
229+
* Smallest integer greater than or equal to "value". Truncation toward zero
230230
* is already the ceiling for a negative raw, so only positive remainders round
231231
* away from zero.
232232
*/

0 commit comments

Comments
 (0)