Skip to content

Commit f7ddf43

Browse files
author
Bo-Yu Cheng
committed
observatory: reviewer-friction fixes (typo, fail-soft, export summary)
- backends/xnnpack + LENSES.md: correct '--lense_recipe' typo to '--lens-recipe' (4 occurrences) so the documented commands actually work as pasted. - devtools/fx_viewer/exporter.py + devtools/observatory/lenses/graph.py: GraphLens now fails soft when 'fast-sugiyama' is missing. The exporter raises a specific _FastSugiyamaMissingError (subclass of ImportError) at the layout site; GraphLens.observe() catches it, logs the actionable 'pip install fast-sugiyama[all]' hint exactly once per process, and returns None for that record. Other lenses continue producing records; the run finishes with exit 0. Restores full graph payloads automatically once the dependency is installed. - devtools/observatory/cli.py: end-of-run export summary via print() (not logging.info, which is swallowed at the default WARNING level). Applies to run_observatory, run_compare, and run_visualize. Lists output paths with byte-size and the collected record names so the CLI no longer ends silently after the last 'Running fast-sugiyama layout...' line. Authored with Claude.
1 parent 785f21b commit f7ddf43

5 files changed

Lines changed: 94 additions & 22 deletions

File tree

backends/xnnpack/debugger/observatory/README.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ python -m executorch.backends.xnnpack.debugger.observatory \
1515

1616
```bash
1717
python -m executorch.backends.xnnpack.debugger.observatory \
18-
--lense_recipe=accuracy \
18+
--lens-recipe=accuracy \
1919
[--output-html PATH] [--output-archive PATH] \
2020
SCRIPT [SCRIPT_ARGS...]
2121
```
@@ -39,7 +39,7 @@ uses `runpy.run_module` instead of `runpy.run_path`.
3939
```bash
4040
python -m executorch.backends.xnnpack.debugger.observatory \
4141
--output-html /tmp/mv2/report.html \
42-
--lense_recipe=accuracy \
42+
--lens-recipe=accuracy \
4343
examples/xnnpack/aot_compiler.py \
4444
--model_name=mv2 --delegate --quantize --output_dir /tmp/mv2
4545
```
@@ -49,7 +49,7 @@ python -m executorch.backends.xnnpack.debugger.observatory \
4949
```bash
5050
python -m executorch.backends.xnnpack.debugger.observatory \
5151
--output-html /tmp/mv2/report.html \
52-
--lense_recipe=accuracy \
52+
--lens-recipe=accuracy \
5353
examples.xnnpack.aot_compiler \
5454
--model_name=mv2 --delegate --quantize --output_dir /tmp/mv2
5555
```
@@ -79,7 +79,7 @@ Pass `--model_name` with any of the models defined in `examples/xnnpack/__init__
7979
Common flags: `--delegate` (XNNPACK delegation, on by default), `--quantize` (8-bit PTQ),
8080
`--output_dir` (where the `.pte` is written).
8181

82-
## Accuracy lenses (`--lense_recipe=accuracy`)
82+
## Accuracy lenses (`--lens-recipe=accuracy`)
8383

8484
Registers `AccuracyLens` and `PerLayerAccuracyLens` on top of the default
8585
`PipelineGraphCollectorLens`. These produce:

devtools/fx_viewer/exporter.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,15 @@
2626
)
2727

2828

29+
class _FastSugiyamaMissingError(ImportError):
30+
"""Raised when the optional fast-sugiyama dependency is not installed.
31+
32+
Subclasses ImportError so callers can catch either this specific type
33+
(to fail-soft, e.g. skip the graph payload for one record) or the
34+
generic ImportError (to fail-fast at library load time).
35+
"""
36+
37+
2938
class FXGraphExporter:
3039
"""Export PyTorch FX graphs to JSON/JS/HTML payloads for the viewer.
3140
@@ -330,7 +339,7 @@ def _compute_layout_with_ext_lines(
330339
try:
331340
from fast_sugiyama import from_edges
332341
except ImportError as exc:
333-
raise ImportError(
342+
raise _FastSugiyamaMissingError(
334343
"fx_viewer layout requires 'fast-sugiyama' (and rectangle-packer "
335344
"for multi-component packing). Install with: "
336345
"pip install 'fast-sugiyama[all]' (requires Python >= 3.11)"

devtools/observatory/cli.py

Lines changed: 53 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,43 @@
4242
# ---------------------------------------------------------------------------
4343

4444

45+
def _format_bytes(n: int) -> str:
46+
for unit in ("B", "KB", "MB", "GB"):
47+
if n < 1024 or unit == "GB":
48+
return f"{n:,.1f} {unit}" if unit != "B" else f"{n:,} B"
49+
n = n / 1024
50+
return f"{n:,.1f} GB"
51+
52+
53+
def _print_export_summary(
54+
output_html: str,
55+
output_archive: str,
56+
output_report_json: str | None,
57+
record_names: list[str],
58+
) -> None:
59+
"""Print a one-shot end-of-run summary so users can see what was written.
60+
61+
Uses ``print`` rather than ``logging.info`` because the default Python log
62+
level is WARNING, which silently swallows info-level messages. A visible
63+
summary matters here: without it, the CLI ends immediately after the last
64+
"Running fast-sugiyama layout..." line, which reads like the run hung.
65+
"""
66+
lines = ["[Observatory CLI] Session complete. Wrote:"]
67+
for path in (output_html, output_archive, output_report_json):
68+
if not path:
69+
continue
70+
try:
71+
size = os.path.getsize(path)
72+
size_str = _format_bytes(size)
73+
except OSError:
74+
size_str = "missing"
75+
lines.append(f" {path} ({size_str})")
76+
lines.append(
77+
f" {len(record_names)} record(s): {', '.join(record_names)}"
78+
)
79+
print("\n".join(lines))
80+
81+
4582
def make_collect_parser(prog=None):
4683
"""Create the base argparse parser for collection mode.
4784
@@ -173,10 +210,11 @@ def run_visualize(input_archive: str, output_html: str, *, setup_fn=None) -> Non
173210
if setup_fn is not None:
174211
setup_fn(Observatory)
175212
Observatory.generate_html_from_json(input_archive, output_html)
176-
logging.info(
177-
"[Observatory CLI] visualize: html=%s from archive=%s",
178-
output_html,
179-
input_archive,
213+
_print_export_summary(
214+
output_html=output_html,
215+
output_archive="", # visualize consumes an archive, does not produce one
216+
output_report_json=None,
217+
record_names=[f"←{os.path.basename(input_archive)}"],
180218
)
181219

182220

@@ -286,15 +324,14 @@ def run_observatory(
286324
Observatory.export_report_json(output_report_json, title=title)
287325
collected = Observatory.list_collected()
288326
if collected:
289-
logging.info(
290-
"[Observatory CLI] Reports: html=%s archive=%s (%d records: %s)",
291-
output_html,
292-
output_archive,
293-
len(collected),
294-
", ".join(collected),
327+
_print_export_summary(
328+
output_html=output_html,
329+
output_archive=output_archive,
330+
output_report_json=output_report_json,
331+
record_names=collected,
295332
)
296333
else:
297-
logging.warning("[Observatory CLI] No records collected")
334+
print("[Observatory CLI] No records collected — no report written.")
298335

299336

300337
# ---------------------------------------------------------------------------
@@ -341,10 +378,11 @@ def run_compare(
341378
# config is not threaded through compare mode (compare_archives drives
342379
# analysis). Pass None so export_report_json uses an empty config.
343380
Observatory.export_report_json(output_report_json, title=title, config=None)
344-
logging.info(
345-
"[Observatory CLI] compare: html=%s from %d archives",
346-
output_html,
347-
len(input_archives),
381+
_print_export_summary(
382+
output_html=output_html,
383+
output_archive="", # compare has no output archive of its own
384+
output_report_json=output_report_json,
385+
record_names=[f"{lbl}{os.path.basename(pth)}" for lbl, pth in zip(labels, input_archives)],
348386
)
349387

350388

devtools/observatory/lenses/LENSES.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ evaluator = StandardEvaluator(
392392

393393
### Disabling Accuracy Evaluation
394394

395-
Accuracy lenses are opt-in via `--lense_recipe=accuracy` on backend CLIs. When
395+
Accuracy lenses are opt-in via `--lens-recipe=accuracy` on backend CLIs. When
396396
omitted, no accuracy evaluation runs. To disable accuracy when using the Python
397397
API directly:
398398

devtools/observatory/lenses/graph.py

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,26 @@
66

77
from __future__ import annotations
88

9+
import logging
910
from typing import Any, Dict, Optional
1011

1112
import torch
1213

1314
from executorch.devtools.fx_viewer import FXGraphExporter
15+
from executorch.devtools.fx_viewer.exporter import _FastSugiyamaMissingError
1416

1517
from ..interfaces import Frontend, GraphView, Lens, ObservationContext, ViewList
1618

1719

1820
class GraphLens(Lens):
1921
"""Canonical producer of base fx_viewer graph payload per record."""
2022

23+
# Emit the missing-fast-sugiyama install hint at most once per process.
24+
# Missing this dep degrades graph payloads to None but does not abort the
25+
# run — all other lenses (accuracy, metadata, adb, ...) continue producing
26+
# their records.
27+
_warned_fast_sugiyama_missing: bool = False
28+
2129
@classmethod
2230
def get_name(cls) -> str:
2331
return "graph"
@@ -57,7 +65,24 @@ def observe(cls, artifact: Any, context: ObservationContext) -> Any:
5765
return None
5866

5967
exporter = FXGraphExporter(graph_module)
60-
payload = exporter.generate_json_payload()
68+
try:
69+
payload = exporter.generate_json_payload()
70+
except _FastSugiyamaMissingError as exc:
71+
# First occurrence — log the actionable install hint once.
72+
# Subsequent occurrences degrade silently to "no graph payload"
73+
# for this record; the run continues, other lenses still fire,
74+
# and Archive/Report artifacts are still produced (just without
75+
# interactive graphs on records that flow through GraphLens).
76+
if not GraphLens._warned_fast_sugiyama_missing:
77+
GraphLens._warned_fast_sugiyama_missing = True
78+
logging.warning(
79+
"[Observatory] GraphLens: %s Records that would carry an "
80+
"interactive FX graph will be captured without one; other "
81+
"lenses continue normally. This message is shown once per "
82+
"process.",
83+
exc,
84+
)
85+
return None
6186

6287
base = payload.get("base", {})
6388
record_name = str(context.shared_state.get("record_name") or "record")

0 commit comments

Comments
 (0)