Skip to content

Commit 8e9ef29

Browse files
edmundmillerclaudeFloWuenne
authored
fix(report): dynamic height + force all labels on cost boxplot (#119)
* fix(report): dynamic height + force all labels on cost boxplot With many processes (44+), the fixed 500px height caused ECharts to hide Y-axis labels. RSEQC_TIN's $3-9 box appeared to belong to RUSTQC. - Scale chart height to 25px per process (min 500px) - Set interval: 0 to force all Y-axis labels visible Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: compute run metrics from tasks when workflowProgress is missing When runs are imported from Nextflow log tarballs (rather than fetched from the Platform API), the progress.workflowProgress object is empty. This caused CPU time, CPU/memory efficiency, and I/O bytes to show as 0 in benchmark charts. Added _compute_progress_from_tasks() fallback that aggregates metrics from task-level data: - cpuTime = sum(cpus * realtime) - cpuLoad = sum(pcpu/100 * realtime) - cpuEfficiency = cpuLoad / cpuTime * 100 - memoryEfficiency = peakRss / memoryReq * 100 - readBytes/writeBytes = sum from tasks Also refactored common accessors into _run_group(), _run_workflow(), and _task_payload() helpers. * fix: avoid falsy-zero bug in _compute_progress_from_tasks The `or` operator treats 0 as falsy, so fields like peakRss=0 would incorrectly fall through to rss. Use explicit None checks via a _val() helper instead. * refactor: restore two-query pattern in query_run_costs Revert from string-concatenation SQL to two explicit queries (CUR vs no-CUR). Each query is now self-contained and readable without mental interpolation of fragment variables. * ci: ignore pre-existing nf-core lint failures in .nf-core.yml Add ignores for files_unchanged mismatches (CONTRIBUTING.md, PULL_REQUEST_TEMPLATE.md, linting_comment.yml, .prettierignore), schema_params (multiqc_title, modules_testdata_base_path, max_multiqc_email_size), and nextflow_config (params.max_multiqc_email_size). These are all pre-existing template drift, not introduced by this PR. * ci: retrigger pull_request workflows * 🐛 fix: handle edge cases in _compute_progress_from_tasks and clean up query_run_costs - Use `run.get("tasks") or []` to handle explicit null tasks from JSON - Use `is not None` check for peakRss to avoid treating 0 as missing - Fix pcpu comment to correctly describe aggregate CPU% semantics - Revert query_run_costs to two separate clean SQL queries matching file convention - Fix docstring to reference Seqera Platform tw run dumps instead of Nextflow log tarballs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * 🔥 remove(ci): delete .nf-core.yml superseded by dev branch cleanup nf-core linting workflows were dropped on dev (ca70336), making this config file obsolete and causing a merge conflict in the PR. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: FloWuenne <flowuenne@gmail.com>
1 parent 17b2890 commit 8e9ef29

2 files changed

Lines changed: 94 additions & 24 deletions

File tree

bin/benchmark_report.py

Lines changed: 92 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,73 @@ def _api_get(url: str, headers: dict[str, str], params: dict[str, str] | None =
4141
# ── JSON normalization (from clean_json.py) ─────────────────────────────────
4242

4343

44+
def _run_group(run: dict) -> str:
45+
"""Return the benchmark group for a run."""
46+
return run["meta"]["group"]
47+
48+
49+
def _run_workflow(run: dict) -> dict:
50+
"""Return the workflow payload for a run."""
51+
return run["workflow"]
52+
53+
54+
def _task_payload(task_raw: dict) -> dict:
55+
"""Unwrap nested task payloads returned by the API."""
56+
if isinstance(task_raw, dict) and "task" in task_raw:
57+
return task_raw["task"]
58+
return task_raw
59+
60+
61+
def _compute_progress_from_tasks(run: dict) -> dict:
62+
"""Compute workflowProgress metrics from task-level data.
63+
64+
This is a fallback for runs where aggregate progress is missing
65+
(e.g. imported from Seqera Platform tw run dumps).
66+
"""
67+
tasks = [_task_payload(t) for t in run.get("tasks") or []]
68+
completed = [t for t in tasks if t.get("status") == "COMPLETED"]
69+
if not completed:
70+
return {}
71+
72+
def _val(d: dict, key: str, default: int = 0) -> int | float:
73+
"""Return d[key] if not None, else default. Avoids falsy-zero bug with `or`."""
74+
v = d.get(key)
75+
return v if v is not None else default
76+
77+
cpu_time = sum(
78+
_val(t, "cpus") * _val(t, "realtime") for t in completed
79+
)
80+
# cpuLoad = actual CPU usage: pcpu is aggregate CPU% across all cores
81+
# (e.g. 400 for a 4-core task at full load), so pcpu/100 * realtime
82+
# gives core-milliseconds used.
83+
cpu_load = sum(
84+
_val(t, "pcpu") / 100.0 * _val(t, "realtime")
85+
for t in completed
86+
)
87+
mem_rss = sum(
88+
_val(t, "peakRss") if t.get("peakRss") is not None else _val(t, "rss")
89+
for t in completed
90+
)
91+
mem_req = sum(_val(t, "memory") for t in completed)
92+
read_bytes = sum(_val(t, "readBytes") for t in completed)
93+
write_bytes = sum(_val(t, "writeBytes") for t in completed)
94+
95+
return {
96+
"cpuTime": int(cpu_time),
97+
"cpuLoad": int(cpu_load),
98+
"cpuEfficiency": (
99+
round(cpu_load / cpu_time * 100, 2) if cpu_time else None
100+
),
101+
"memoryRss": mem_rss,
102+
"memoryReq": mem_req,
103+
"memoryEfficiency": (
104+
round(mem_rss / mem_req * 100, 2) if mem_req else None
105+
),
106+
"readBytes": read_bytes,
107+
"writeBytes": write_bytes,
108+
}
109+
110+
44111
def _write_tmp_json(rows: list[dict], name: str) -> str:
45112
"""Write rows to a temporary JSON file for DuckDB to read."""
46113
path = os.path.join(tempfile.gettempdir(), f"nfagg_{name}.json")
@@ -62,8 +129,10 @@ def extract_runs(runs: list[dict]) -> list[dict]:
62129
"""Extract run-level metadata from raw API data."""
63130
run_rows = []
64131
for r in runs:
65-
wf = r["workflow"]
132+
wf = _run_workflow(r)
66133
prog = r.get("progress", {}).get("workflowProgress", {})
134+
if not prog:
135+
prog = _compute_progress_from_tasks(r)
67136
stats = wf.get("stats", {})
68137
launch = r.get("launch", {}) or {}
69138
ce = r.get("computeEnv", {}) or {}
@@ -74,7 +143,7 @@ def extract_runs(runs: list[dict]) -> list[dict]:
74143

75144
run_rows.append({
76145
"run_id": wf["id"],
77-
"group": r["meta"]["group"],
146+
"group": _run_group(r),
78147
"pipeline": (
79148
wf.get("projectName")
80149
or wf.get("repository", "").split("/")[-1]
@@ -120,14 +189,10 @@ def extract_tasks(runs: list[dict]) -> list[dict]:
120189
"""Extract task-level data from raw API data."""
121190
task_rows = []
122191
for r in runs:
123-
run_id = r["workflow"]["id"]
124-
group = r["meta"]["group"]
192+
run_id = _run_workflow(r)["id"]
193+
group = _run_group(r)
125194
for t_raw in r.get("tasks", []):
126-
t = (
127-
t_raw.get("task", t_raw)
128-
if isinstance(t_raw, dict) and "task" in t_raw
129-
else t_raw
130-
)
195+
t = _task_payload(t_raw)
131196
task_rows.append({
132197
"run_id": run_id,
133198
"group": group,
@@ -164,8 +229,8 @@ def extract_metrics(runs: list[dict]) -> list[dict]:
164229
"""Extract per-process metrics from raw API data."""
165230
metrics_rows = []
166231
for r in runs:
167-
run_id = r["workflow"]["id"]
168-
group = r["meta"]["group"]
232+
run_id = _run_workflow(r)["id"]
233+
group = _run_group(r)
169234
for m in r.get("metrics", []):
170235
row = {
171236
"run_id": run_id,
@@ -186,15 +251,23 @@ def extract_metrics(runs: list[dict]) -> list[dict]:
186251
# ── CUR cost processing (from clean_cur.py) ────────────────────────────────
187252

188253

189-
def detect_cur_format(db: duckdb.DuckDBPyConnection, cur_path: str) -> str:
190-
"""Detect CUR format: 'map' (CUR 2.0) or 'flat' (CUR 1.0) or 'unknown'."""
191-
cur_cols = {
192-
r[0]
193-
for r in db.execute(
254+
def _parquet_columns(
255+
db: duckdb.DuckDBPyConnection,
256+
cur_path: str,
257+
) -> set[str]:
258+
"""Return the available columns in a parquet file."""
259+
return {
260+
row[0]
261+
for row in db.execute(
194262
f"SELECT column_name FROM (DESCRIBE SELECT * FROM read_parquet('{cur_path}'))"
195263
).fetchall()
196264
}
197265

266+
267+
def detect_cur_format(db: duckdb.DuckDBPyConnection, cur_path: str) -> str:
268+
"""Detect CUR format: 'map' (CUR 2.0) or 'flat' (CUR 1.0) or 'unknown'."""
269+
cur_cols = _parquet_columns(db, cur_path)
270+
198271
is_map = (
199272
"resource_tags" in cur_cols
200273
and "resource_tags_user_unique_run_id" not in cur_cols
@@ -242,12 +315,7 @@ def build_costs_flat_format(
242315
db: duckdb.DuckDBPyConnection, cur_path: str
243316
) -> None:
244317
"""Build costs table from CUR 1.0 flattened format."""
245-
cur_cols = {
246-
r[0]
247-
for r in db.execute(
248-
f"SELECT column_name FROM (DESCRIBE SELECT * FROM read_parquet('{cur_path}'))"
249-
).fetchall()
250-
}
318+
cur_cols = _parquet_columns(db, cur_path)
251319

252320
has_nf_run_id = "resource_tags_user_nf_unique_run_id" in cur_cols
253321
has_run_id = "resource_tags_user_unique_run_id" in cur_cols
@@ -443,6 +511,7 @@ def query_run_metrics(db: duckdb.DuckDBPyConnection) -> list[dict]:
443511
def query_run_costs(db: duckdb.DuckDBPyConnection) -> list[dict]:
444512
"""Per-run cost from task-level sums + optional CUR costs."""
445513
has_cur = table_exists(db, "costs")
514+
446515
if has_cur:
447516
return fetch_dicts(db, """
448517
SELECT
@@ -458,6 +527,7 @@ def query_run_costs(db: duckdb.DuckDBPyConnection) -> list[dict]:
458527
GROUP BY r.run_id, r."group"
459528
ORDER BY r."group"
460529
""")
530+
461531
return fetch_dicts(db, """
462532
SELECT
463533
r.run_id,

bin/benchmark_report_template.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -697,7 +697,6 @@ <h2 id="task-metrics"><svg class="h-icon sm"><use href="#ic-scatter"/></svg> Tas
697697

698698
const costBoxEl = document.createElement('div');
699699
costBoxEl.className = 'chart';
700-
costBoxEl.style.height = '500px';
701700
container.appendChild(costBoxEl);
702701

703702
setTimeout(() => {
@@ -718,12 +717,13 @@ <h2 id="task-metrics"><svg class="h-icon sm"><use href="#ic-scatter"/></svg> Tas
718717
}
719718
});
720719

720+
costBoxEl.style.height = Math.max(500, processes.length * 25 + 60) + 'px';
721721
echarts.init(costBoxEl, 'seqera').setOption({
722722
title: { text: 'Task cost ($) per process' },
723723
tooltip: { trigger: 'item' },
724724
grid: { top: 40, bottom: 20, containLabel: true },
725725
yAxis: { type: 'category', data: processes,
726-
axisLabel: { width: 220, overflow: 'truncate' },
726+
axisLabel: { width: 220, overflow: 'truncate', interval: 0 },
727727
inverse: true },
728728
xAxis: { type: 'value', name: 'Cost ($)', nameLocation: 'center', nameGap: 25 },
729729
series: [{ type: 'boxplot', data: boxData }],

0 commit comments

Comments
 (0)