Skip to content

Commit 5a975e1

Browse files
FloWuenneclaude
andcommitted
🐛 fix(cost): pool the spot/on-demand split across a resumed session too
The spot coverage card read $0.014 spot beside a $256 session total: costs were pooled across a resumed run's attempts but the purchase-option split was not, so it priced only the last attempt's own machine rows. The machines that ran the cached tasks were rented by an earlier attempt. _purchase_option_split now takes the same pool the costs come from — the whole session for a resumed run — so the split describes the money shown beside it. That exposed a second problem introduced with session-wide costs: when two attempts of one session are both in the report, the older one's spend is inside the newer one's session figure, so any total counted it twice. Runs now carry superseded_cost, and the fleet spot totals, the total cost card and the engine split rows skip those rows. The row itself still shows what that attempt spent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 476cd8c commit 5a975e1

4 files changed

Lines changed: 141 additions & 111 deletions

File tree

‎bin/benchmark_report_ic_aggregate.py‎

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,10 @@ def _classify_missing_cost(reference_ts: Any, now: datetime) -> str:
215215
def _purchase_option_split(
216216
detail: dict[str, Any] | None, compute_type: str
217217
) -> tuple[float | None, float | None, float | None]:
218-
"""Spot vs on-demand machine spend for one run — ``(spot, ondemand, spot_pct)``.
218+
"""Spot vs on-demand machine spend — ``(spot, ondemand, spot_pct)``.
219+
220+
``detail`` is a cost pool: a run's own rows, or (for a resumed run) its whole session's,
221+
so the split always describes the same money as the cost shown beside it.
219222
220223
ALWAYS the unblended basis, on both Intelligent Compute architectures, so the figure means
221224
the same thing in every row it appears in. The ECS split basis is not an option here: AWS
@@ -338,18 +341,19 @@ def build_ic_report_data(
338341
req_mem_gib = round(res.get("mem_req", 0.0), 1)
339342
eff_mem_gib = round(res.get("mem_used", 0.0), 1)
340343

341-
# RESUMED RUNS: every figure in this row except the session_* pair is this ATTEMPT only,
342-
# and stays that way on purpose. Each one is either divided by, or read next to, a
343-
# denominator that describes the last attempt alone — compute_hours and req/eff vCPU-h
344-
# come from this run's task records, wall_time_ms from this run's duration, and the
345-
# machines CSV (when supplied) lists this run's instances. Pooling cost across attempts
346-
# into those ratios would put lineage money over last-attempt capacity and quietly break
347-
# $/vCPU-h, spot coverage and every utilisation figure.
344+
# RESUMED RUNS: every MONEY figure is the whole session — what all of its attempts spent
345+
# together, since the cached tasks this attempt reused were paid for by an earlier one.
346+
# That is what the report shows and compares against AWS Batch, so the purchase-option
347+
# split has to come from the same pool: a session figure beside an attempt's spot cost
348+
# reads as "$256 of which $0.01 was spot".
349+
#
350+
# Capacity figures stay attempt-scoped and MUST NOT be mixed with these: compute_hours
351+
# and req/eff vCPU-h come from this run's task records, wall_time_ms from its duration,
352+
# the machines CSV from its instances. Any ratio of the two (e.g. $/vCPU-h) would put
353+
# lineage money over last-attempt capacity.
348354
#
349-
# The lineage total therefore lives in its own fields, reported as "total across N
350-
# attempts" rather than mixed into the run's own numbers. Only the newest attempt of a
351-
# session gets them, so listing several attempts of one session cannot count the same
352-
# dollars twice.
355+
# Session pools go only to the NEWEST attempt of a session, so listing several attempts
356+
# of one session cannot count the same dollars twice.
353357
session_id = lineage.get(run_id, {}).get("session_id", "")
354358
session_pool = (
355359
pools["by_session"].get(session_id) if session_owners.get(session_id) == run_id else None
@@ -390,8 +394,6 @@ def build_ic_report_data(
390394
run.get("complete") or started_at, now
391395
)
392396

393-
spot_cost, ondemand_cost, spot_pct = _purchase_option_split(detail, compute_type)
394-
395397
if session_pool is not None:
396398
billed_attempts = pools["session_attempts"].get(session_id, set())
397399
attempts = max(len(billed_attempts), 1)
@@ -411,6 +413,15 @@ def build_ic_report_data(
411413
earlier_attempts = 0
412414
session_cost = session_comparable_cost = None
413415

416+
# Purchase option on the SAME whole-session basis as the costs above. For a resumed run
417+
# the machines that ran its cached tasks were rented by an earlier attempt, so splitting
418+
# only this attempt's rows priced a few cents of spot against a session that spent
419+
# dollars — and the report's headline cost, which is the session's, then had no matching
420+
# spot figure. `by_session` sums the same spot/ondemand fields across every attempt.
421+
spot_cost, ondemand_cost, spot_pct = _purchase_option_split(
422+
session_pool if session_pool is not None else detail, compute_type
423+
)
424+
414425
run_summary.append({
415426
"run_id": run_id,
416427
"run_url": run_url,
@@ -449,7 +460,8 @@ def build_ic_report_data(
449460
"unused_cost": unused_cost,
450461
# Machine spend split by EC2 purchase option, unblended basis, Intelligent Compute
451462
# only (see _purchase_option_split). None for AWS Batch and for IC runs with no
452-
# machine rows. spot_cost + ondemand_cost is machine spend, a SUBSET of "cost".
463+
# machine rows. spot_cost + ondemand_cost is machine spend, a SUBSET of the billed
464+
# cost on the same scope — the whole session for a resumed run, matching session_cost.
453465
"spot_cost": spot_cost,
454466
"ondemand_cost": ondemand_cost,
455467
"spot_pct": spot_pct,
@@ -465,6 +477,12 @@ def build_ic_report_data(
465477
"earlier_attempts": earlier_attempts,
466478
"session_cost": session_cost,
467479
"session_comparable_cost": session_comparable_cost,
480+
# True when this attempt is NOT the one carrying its session's pooled figures, i.e.
481+
# a newer attempt of the same session is also in this report and its session_cost
482+
# already contains this attempt's spend. Every total must skip these rows or the
483+
# same dollars land in it twice; the row itself still shows its own attempt's cost,
484+
# which is what that attempt really spent.
485+
"superseded_cost": bool(session_id) and session_owners.get(session_id) not in (None, run_id),
468486
"cached_tasks": lineage.get(run_id, {}).get("cached", 0),
469487
"executed_tasks": lineage.get(run_id, {}).get("succeeded", 0),
470488
})
@@ -499,7 +517,9 @@ def build_ic_report_data(
499517
# basis as the per-run figures. Weighted by spend, not a mean of per-run percentages, so a
500518
# cheap all-spot run cannot outvote an expensive fallback. None when no IC run has machine
501519
# rows, which is what hides the stat card rather than showing a misleading 0%.
502-
spot_runs = [r for r in run_summary if r["spot_cost"] is not None]
520+
spot_runs = [
521+
r for r in run_summary if r["spot_cost"] is not None and not r["superseded_cost"]
522+
]
503523
spot_total = round(sum(r["spot_cost"] for r in spot_runs), 4)
504524
ondemand_total = round(sum(r["ondemand_cost"] for r in spot_runs), 4)
505525
machine_total = spot_total + ondemand_total

‎bin/benchmark_report_ic_template.html‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -549,8 +549,11 @@ <h2 class="section-title" id="cost-title">Cost</h2>
549549
// split figure, while AWS Batch only ever has the split figure. Pairing "IC billed" against
550550
// "Batch billed" would set a real charge against a permanent em-dash; pairing the two split
551551
// rows is the like-for-like read, with IC's billed total above it as the real spend.
552-
const billed = runs.filter((r) => totalCost(r) != null);
553-
const comparable = runs.filter((r) => comparableCost(r) != null);
552+
// Sums skip superseded attempts: an older attempt of a resumed session whose spend is
553+
// already inside the newer attempt's session figure. Adding both would count it twice.
554+
const counted = runs.filter((r) => !r.superseded_cost);
555+
const billed = counted.filter((r) => totalCost(r) != null);
556+
const comparable = counted.filter((r) => comparableCost(r) != null);
554557
if (billed.length || comparable.length) {
555558
const sum = (rows, pick) => rows.reduce((s, r) => s + Number(pick(r)), 0);
556559
const has = (type) => runs.some((r) => r.compute_type === type);
@@ -563,11 +566,11 @@ <h2 class="section-title" id="cost-title">Cost</h2>
563566
// silently omit every AWS Batch run and read as the whole spend, which is worse than mixing
564567
// bases — but it IS a mix, so the card label says so.
565568
const totalFor = (r) => (totalCost(r) != null ? totalCost(r) : comparableCost(r));
566-
const counted = runs.filter((r) => totalFor(r) != null);
567-
if (counted.length) {
569+
const inTotal = counted.filter((r) => totalFor(r) != null);
570+
if (inTotal.length) {
568571
const el = document.querySelector("#stat-cost .n");
569572
el.classList.remove("muted");
570-
el.textContent = fmtCost(sum(counted, totalFor));
573+
el.textContent = fmtCost(sum(inTotal, totalFor));
571574
}
572575

573576
const row = (dot, label, value, title) =>

‎modules/local/aggregate_ic_report_data/bin/benchmark_report_ic_aggregate.py‎

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,10 @@ def _classify_missing_cost(reference_ts: Any, now: datetime) -> str:
215215
def _purchase_option_split(
216216
detail: dict[str, Any] | None, compute_type: str
217217
) -> tuple[float | None, float | None, float | None]:
218-
"""Spot vs on-demand machine spend for one run — ``(spot, ondemand, spot_pct)``.
218+
"""Spot vs on-demand machine spend — ``(spot, ondemand, spot_pct)``.
219+
220+
``detail`` is a cost pool: a run's own rows, or (for a resumed run) its whole session's,
221+
so the split always describes the same money as the cost shown beside it.
219222
220223
ALWAYS the unblended basis, on both Intelligent Compute architectures, so the figure means
221224
the same thing in every row it appears in. The ECS split basis is not an option here: AWS
@@ -338,18 +341,19 @@ def build_ic_report_data(
338341
req_mem_gib = round(res.get("mem_req", 0.0), 1)
339342
eff_mem_gib = round(res.get("mem_used", 0.0), 1)
340343

341-
# RESUMED RUNS: every figure in this row except the session_* pair is this ATTEMPT only,
342-
# and stays that way on purpose. Each one is either divided by, or read next to, a
343-
# denominator that describes the last attempt alone — compute_hours and req/eff vCPU-h
344-
# come from this run's task records, wall_time_ms from this run's duration, and the
345-
# machines CSV (when supplied) lists this run's instances. Pooling cost across attempts
346-
# into those ratios would put lineage money over last-attempt capacity and quietly break
347-
# $/vCPU-h, spot coverage and every utilisation figure.
344+
# RESUMED RUNS: every MONEY figure is the whole session — what all of its attempts spent
345+
# together, since the cached tasks this attempt reused were paid for by an earlier one.
346+
# That is what the report shows and compares against AWS Batch, so the purchase-option
347+
# split has to come from the same pool: a session figure beside an attempt's spot cost
348+
# reads as "$256 of which $0.01 was spot".
349+
#
350+
# Capacity figures stay attempt-scoped and MUST NOT be mixed with these: compute_hours
351+
# and req/eff vCPU-h come from this run's task records, wall_time_ms from its duration,
352+
# the machines CSV from its instances. Any ratio of the two (e.g. $/vCPU-h) would put
353+
# lineage money over last-attempt capacity.
348354
#
349-
# The lineage total therefore lives in its own fields, reported as "total across N
350-
# attempts" rather than mixed into the run's own numbers. Only the newest attempt of a
351-
# session gets them, so listing several attempts of one session cannot count the same
352-
# dollars twice.
355+
# Session pools go only to the NEWEST attempt of a session, so listing several attempts
356+
# of one session cannot count the same dollars twice.
353357
session_id = lineage.get(run_id, {}).get("session_id", "")
354358
session_pool = (
355359
pools["by_session"].get(session_id) if session_owners.get(session_id) == run_id else None
@@ -390,8 +394,6 @@ def build_ic_report_data(
390394
run.get("complete") or started_at, now
391395
)
392396

393-
spot_cost, ondemand_cost, spot_pct = _purchase_option_split(detail, compute_type)
394-
395397
if session_pool is not None:
396398
billed_attempts = pools["session_attempts"].get(session_id, set())
397399
attempts = max(len(billed_attempts), 1)
@@ -411,6 +413,15 @@ def build_ic_report_data(
411413
earlier_attempts = 0
412414
session_cost = session_comparable_cost = None
413415

416+
# Purchase option on the SAME whole-session basis as the costs above. For a resumed run
417+
# the machines that ran its cached tasks were rented by an earlier attempt, so splitting
418+
# only this attempt's rows priced a few cents of spot against a session that spent
419+
# dollars — and the report's headline cost, which is the session's, then had no matching
420+
# spot figure. `by_session` sums the same spot/ondemand fields across every attempt.
421+
spot_cost, ondemand_cost, spot_pct = _purchase_option_split(
422+
session_pool if session_pool is not None else detail, compute_type
423+
)
424+
414425
run_summary.append({
415426
"run_id": run_id,
416427
"run_url": run_url,
@@ -449,7 +460,8 @@ def build_ic_report_data(
449460
"unused_cost": unused_cost,
450461
# Machine spend split by EC2 purchase option, unblended basis, Intelligent Compute
451462
# only (see _purchase_option_split). None for AWS Batch and for IC runs with no
452-
# machine rows. spot_cost + ondemand_cost is machine spend, a SUBSET of "cost".
463+
# machine rows. spot_cost + ondemand_cost is machine spend, a SUBSET of the billed
464+
# cost on the same scope — the whole session for a resumed run, matching session_cost.
453465
"spot_cost": spot_cost,
454466
"ondemand_cost": ondemand_cost,
455467
"spot_pct": spot_pct,
@@ -465,6 +477,12 @@ def build_ic_report_data(
465477
"earlier_attempts": earlier_attempts,
466478
"session_cost": session_cost,
467479
"session_comparable_cost": session_comparable_cost,
480+
# True when this attempt is NOT the one carrying its session's pooled figures, i.e.
481+
# a newer attempt of the same session is also in this report and its session_cost
482+
# already contains this attempt's spend. Every total must skip these rows or the
483+
# same dollars land in it twice; the row itself still shows its own attempt's cost,
484+
# which is what that attempt really spent.
485+
"superseded_cost": bool(session_id) and session_owners.get(session_id) not in (None, run_id),
468486
"cached_tasks": lineage.get(run_id, {}).get("cached", 0),
469487
"executed_tasks": lineage.get(run_id, {}).get("succeeded", 0),
470488
})
@@ -499,7 +517,9 @@ def build_ic_report_data(
499517
# basis as the per-run figures. Weighted by spend, not a mean of per-run percentages, so a
500518
# cheap all-spot run cannot outvote an expensive fallback. None when no IC run has machine
501519
# rows, which is what hides the stat card rather than showing a misleading 0%.
502-
spot_runs = [r for r in run_summary if r["spot_cost"] is not None]
520+
spot_runs = [
521+
r for r in run_summary if r["spot_cost"] is not None and not r["superseded_cost"]
522+
]
503523
spot_total = round(sum(r["spot_cost"] for r in spot_runs), 4)
504524
ondemand_total = round(sum(r["ondemand_cost"] for r in spot_runs), 4)
505525
machine_total = spot_total + ondemand_total

0 commit comments

Comments
 (0)