-
Notifications
You must be signed in to change notification settings - Fork 1
update eval scripts: add ONNX size tracking and output sanitization #755
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
0b3eaac
9c152a0
7d00b37
f21a123
b3e0cf9
137ea51
9154eff
2652af1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -31,8 +31,10 @@ | |||||||||||||||||||
| import argparse | ||||||||||||||||||||
| import contextlib | ||||||||||||||||||||
| import json | ||||||||||||||||||||
| import logging | ||||||||||||||||||||
| import os | ||||||||||||||||||||
| import platform | ||||||||||||||||||||
| import re | ||||||||||||||||||||
| import shutil | ||||||||||||||||||||
| import subprocess | ||||||||||||||||||||
| import sys | ||||||||||||||||||||
|
|
@@ -69,6 +71,8 @@ | |||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| logger = logging.getLogger(__name__) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # --------------------------------------------------------------------------- | ||||||||||||||||||||
| # Constants | ||||||||||||||||||||
| # --------------------------------------------------------------------------- | ||||||||||||||||||||
|
|
@@ -233,6 +237,82 @@ def _utc_now() -> str: | |||||||||||||||||||
| return datetime.now(timezone.utc).isoformat() | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| def _compute_onnx_size(onnx_paths: dict[str, str]) -> int | None: | ||||||||||||||||||||
| """Return combined size in bytes of all ONNX files + their external data companions. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Parses the ONNX proto to discover all referenced external data files (not just | ||||||||||||||||||||
| the conventional `.data` suffix). Falls back to the `.data` companion heuristic | ||||||||||||||||||||
| if proto parsing is unavailable. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Returns None if onnx_paths is empty or no files exist on disk. | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| if not onnx_paths: | ||||||||||||||||||||
| return None | ||||||||||||||||||||
| total = 0 | ||||||||||||||||||||
| found_any = False | ||||||||||||||||||||
| for path_str in onnx_paths.values(): | ||||||||||||||||||||
| p = Path(path_str) | ||||||||||||||||||||
| if not p.exists(): | ||||||||||||||||||||
| continue | ||||||||||||||||||||
| total += p.stat().st_size | ||||||||||||||||||||
| found_any = True | ||||||||||||||||||||
| # Try to enumerate all external data files from the proto | ||||||||||||||||||||
| try: | ||||||||||||||||||||
| from winml.modelkit.onnx.external_data import get_external_data_files | ||||||||||||||||||||
|
|
||||||||||||||||||||
| ext_files = get_external_data_files(p) | ||||||||||||||||||||
| for ext_name in ext_files: | ||||||||||||||||||||
| ext_path = p.parent / ext_name | ||||||||||||||||||||
| if ext_path.exists(): | ||||||||||||||||||||
| total += ext_path.stat().st_size | ||||||||||||||||||||
| except Exception: | ||||||||||||||||||||
| # Fallback: check conventional .data companion | ||||||||||||||||||||
| data_p = p.with_suffix(p.suffix + ".data") | ||||||||||||||||||||
| if data_p.exists(): | ||||||||||||||||||||
| total += data_p.stat().st_size | ||||||||||||||||||||
| return total if found_any else None | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| # Lines that carry no diagnostic value in eval_result.json. | ||||||||||||||||||||
| # Matching is case-insensitive, anchored at line start. | ||||||||||||||||||||
| _NOISE_PATTERNS = ( | ||||||||||||||||||||
| "benchmarking onnx", | ||||||||||||||||||||
| "device:", | ||||||||||||||||||||
| "task:", | ||||||||||||||||||||
| "latency (ms)", | ||||||||||||||||||||
| "throughput:", | ||||||||||||||||||||
| "results saved to", | ||||||||||||||||||||
|
DingmaomaoBJTU marked this conversation as resolved.
|
||||||||||||||||||||
| "inputs:", | ||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A couple of patterns in
Tightening options (cheapest first):
🤖 Generated with GitHub Copilot CLI |
||||||||||||||||||||
| "outputs:", | ||||||||||||||||||||
| "samples/sec", | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| _NOISE_RE = re.compile("|".join(re.escape(p) for p in _NOISE_PATTERNS), re.IGNORECASE) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # Box-drawing characters used by Rich tables. | ||||||||||||||||||||
| _BOX_CHARS = frozenset("─│┌┐└┘├┤┬┴┼") | ||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Net result: Cheap fix — use a Unicode block range check instead of a hand-curated set: if 0x2500 <= ord(stripped[0]) <= 0x257F: # Unicode "Box Drawing" block
continueThat covers all four Rich styles (light, heavy, double, rounded) in one rule and won''t silently drift the next time someone changes the table style. 🤖 Generated with GitHub Copilot CLI |
||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| def _sanitize_output(text: str) -> str: | ||||||||||||||||||||
| """Strip routine CLI chrome from subprocess output, keeping error content. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Removes Rich benchmark tables, device/IO banners, and path lines that | ||||||||||||||||||||
| bloat eval_result.json without aiding failure diagnosis. All classifier | ||||||||||||||||||||
| patterns (see classifier.py) are error-related and survive this filter. | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| kept: list[str] = [] | ||||||||||||||||||||
| for line in text.splitlines(): | ||||||||||||||||||||
| stripped = line.strip() | ||||||||||||||||||||
| if not stripped: | ||||||||||||||||||||
| continue | ||||||||||||||||||||
|
DingmaomaoBJTU marked this conversation as resolved.
|
||||||||||||||||||||
| # Drop box-drawing table rows | ||||||||||||||||||||
| if stripped[0] in _BOX_CHARS: | ||||||||||||||||||||
| continue | ||||||||||||||||||||
| if _NOISE_RE.match(stripped): | ||||||||||||||||||||
| continue | ||||||||||||||||||||
| kept.append(stripped) | ||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Appending becomes a visually-flat: which is noticeably harder to read. Suggested fix — keep if not stripped:
continue
if 0x2500 <= ord(stripped[0]) <= 0x257F:
continue
low = stripped.lower()
if any(low.startswith(pat) for pat in _NOISE_PATTERNS):
continue
kept.append(line.rstrip())🤖 Generated with GitHub Copilot CLI |
||||||||||||||||||||
| return "\n".join(kept) | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| def _kill_process_tree(pid: int) -> None: | ||||||||||||||||||||
| """Kill a process and all its children. | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
DingmaomaoBJTU marked this conversation as resolved.
|
||||||||||||||||||||
|
|
@@ -1028,6 +1108,20 @@ def save_environment_info(path: Path) -> None: | |||||||||||||||||||
| except (subprocess.TimeoutExpired, FileNotFoundError): | ||||||||||||||||||||
| pass # git not available or timed out; commit info stays empty | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # `winml sys --format json` captures hardware details (devices, EPs, | ||||||||||||||||||||
| # backends) that the lightweight package-version probes above miss. | ||||||||||||||||||||
| try: | ||||||||||||||||||||
| result = subprocess.run( # noqa: S603 | ||||||||||||||||||||
| [sys.executable, "-m", "winml", "sys", "--format", "json"], | ||||||||||||||||||||
| capture_output=True, | ||||||||||||||||||||
| text=True, | ||||||||||||||||||||
| timeout=30, | ||||||||||||||||||||
|
DingmaomaoBJTU marked this conversation as resolved.
|
||||||||||||||||||||
| ) | ||||||||||||||||||||
| if result.returncode == 0: | ||||||||||||||||||||
| info["winml_sys"] = json.loads(result.stdout) | ||||||||||||||||||||
|
DingmaomaoBJTU marked this conversation as resolved.
|
||||||||||||||||||||
| except (subprocess.TimeoutExpired, FileNotFoundError, json.JSONDecodeError) as exc: | ||||||||||||||||||||
| logger.debug("winml sys skipped: %s", exc) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| path.parent.mkdir(parents=True, exist_ok=True) | ||||||||||||||||||||
| path.write_text(json.dumps(info, indent=2), encoding="utf-8") | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -1139,6 +1233,11 @@ def parse_args() -> argparse.Namespace: | |||||||||||||||||||
| help="Skip report generation (useful when running per-model in a pipeline loop)", | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| parser.add_argument("--verbose", "-v", action="store_true", help="Verbose output") | ||||||||||||||||||||
| parser.add_argument( | ||||||||||||||||||||
| "--raw-output", | ||||||||||||||||||||
| action="store_true", | ||||||||||||||||||||
| help="Keep raw subprocess output in eval_result.json without sanitization", | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| parser.add_argument( | ||||||||||||||||||||
| "--continue", | ||||||||||||||||||||
| dest="continue_run", | ||||||||||||||||||||
|
|
@@ -1399,6 +1498,7 @@ def main() -> None: | |||||||||||||||||||
| ep=args.ep, | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| onnx_paths = build_result["onnx_paths"] if build_result["success"] else {} | ||||||||||||||||||||
| onnx_size = _compute_onnx_size(onnx_paths) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if not build_result["success"]: | ||||||||||||||||||||
| # Build failed — synthesize failed result for downstream phases | ||||||||||||||||||||
|
|
@@ -1443,7 +1543,14 @@ def main() -> None: | |||||||||||||||||||
| break | ||||||||||||||||||||
|
|
||||||||||||||||||||
| result = build_eval_result( | ||||||||||||||||||||
| entry, perf_proc, args.device, eval_types_run, accuracy_result, ep=args.ep | ||||||||||||||||||||
| entry, | ||||||||||||||||||||
| perf_proc, | ||||||||||||||||||||
| args.device, | ||||||||||||||||||||
| eval_types_run, | ||||||||||||||||||||
| accuracy_result, | ||||||||||||||||||||
| ep=args.ep, | ||||||||||||||||||||
| onnx_size_bytes=onnx_size, | ||||||||||||||||||||
| sanitize_fn=None if args.raw_output else _sanitize_output, | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| results.append(result) | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a little strange.. any way we add a parameter in eval command to just drop them?