flow/designs: per-PDK WNS plots and README from rules-base.json#4307
flow/designs: per-PDK WNS plots and README from rules-base.json#4307oharboe wants to merge 2 commits into
Conversation
Add flow/util/plot_wns.py, a local (no-bazel) script that reads the committed rules-base.json baselines and regenerates, per PDK, a worst-setup-slack bar chart (wns.png) and a "## WNS" README section between generated markers. It covers all 9 PDKs that ship timing baselines (76 designs); the bar is finish-stage WNS, with cts and globalroute drawn as markers so stage-to-stage movement is visible. No OpenROAD/ORFS run is required -- the data already lives in the tree, so the plots are deterministic and the committed PNGs + tables render on GitHub with nothing to run. asap7/README.md additionally carries a hand-written findings section discussing the results. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a Python script, plot_wns.py, to automatically parse worst setup slack (WNS) data from rules-base.json baselines and generate markdown tables and bar charts in the README files of various PDKs. The review feedback highlights three key areas for improvement: adding explicit encoding="utf-8" parameters to file operations to prevent encoding crashes on non-UTF-8 platforms (such as Windows), utilizing the defined FINISH_KEY constant within the STAGES list to eliminate unused code, and adding a type check to ensure the parsed JSON is a dictionary to avoid potential AttributeError crashes.
| if os.path.isfile(readme_path): | ||
| with open(readme_path) as f: | ||
| text = f.read() | ||
| if BEGIN in text and END in text: | ||
| pre = text[: text.index(BEGIN)] | ||
| post = text[text.index(END) + len(END):] | ||
| new = pre + section + post.lstrip("\n") | ||
| else: | ||
| new = text.rstrip("\n") + "\n\n" + section | ||
| else: | ||
| new = f"# {pdk} designs\n\n" + section | ||
| with open(readme_path, "w") as f: | ||
| f.write(new) |
There was a problem hiding this comment.
The generated README files contain non-ASCII characters such as em-dashes (—) and almost-equal-to signs (≈). When running this script on platforms where the default system encoding is not UTF-8 (such as Windows), opening these files without explicitly specifying encoding="utf-8" will cause UnicodeDecodeError or UnicodeEncodeError crashes. Specifying encoding="utf-8" on both read and write operations prevents this.
| if os.path.isfile(readme_path): | |
| with open(readme_path) as f: | |
| text = f.read() | |
| if BEGIN in text and END in text: | |
| pre = text[: text.index(BEGIN)] | |
| post = text[text.index(END) + len(END):] | |
| new = pre + section + post.lstrip("\n") | |
| else: | |
| new = text.rstrip("\n") + "\n\n" + section | |
| else: | |
| new = f"# {pdk} designs\n\n" + section | |
| with open(readme_path, "w") as f: | |
| f.write(new) | |
| if os.path.isfile(readme_path): | |
| with open(readme_path, encoding="utf-8") as f: | |
| text = f.read() | |
| if BEGIN in text and END in text: | |
| pre = text[: text.index(BEGIN)] | |
| post = text[text.index(END) + len(END):] | |
| new = pre + section + post.lstrip("\n") | |
| else: | |
| new = text.rstrip("\n") + "\n\n" + section | |
| else: | |
| new = f"# {pdk} designs\n\n" + section | |
| with open(readme_path, "w", encoding="utf-8") as f: | |
| f.write(new) |
| # Stage key -> (short label, plot marker). Order is flow order. | ||
| STAGES = [ | ||
| ("cts__timing__setup__ws", "cts", "v"), | ||
| ("globalroute__timing__setup__ws", "globalroute", "^"), | ||
| ("finish__timing__setup__ws", "finish", None), # finish is drawn as the bar | ||
| ] | ||
| FINISH_KEY = "finish__timing__setup__ws" |
There was a problem hiding this comment.
The constant FINISH_KEY is defined but never used in the script. It is cleaner and more maintainable to define it first and use it directly in the STAGES list to avoid hardcoding the string twice.
| # Stage key -> (short label, plot marker). Order is flow order. | |
| STAGES = [ | |
| ("cts__timing__setup__ws", "cts", "v"), | |
| ("globalroute__timing__setup__ws", "globalroute", "^"), | |
| ("finish__timing__setup__ws", "finish", None), # finish is drawn as the bar | |
| ] | |
| FINISH_KEY = "finish__timing__setup__ws" | |
| FINISH_KEY = "finish__timing__setup__ws" | |
| # Stage key -> (short label, plot marker). Order is flow order. | |
| STAGES = [ | |
| ("cts__timing__setup__ws", "cts", "v"), | |
| ("globalroute__timing__setup__ws", "globalroute", "^"), | |
| (FINISH_KEY, "finish", None), # finish is drawn as the bar | |
| ] |
| try: | ||
| with open(path) as f: | ||
| data = json.load(f) | ||
| except (OSError, ValueError): | ||
| return None | ||
| entry = data.get(key) | ||
| if isinstance(entry, dict) and isinstance(entry.get("value"), (int, float)): | ||
| return float(entry["value"]) | ||
| return None |
There was a problem hiding this comment.
If rules-base.json is malformed or empty, json.load(f) could return a non-dictionary object (like a list or string). Calling data.get(key) would then raise an AttributeError, which is not caught by the except (OSError, ValueError) block and would crash the script. Additionally, specifying encoding="utf-8" when opening files is a best practice to ensure cross-platform compatibility (especially on Windows).
| try: | |
| with open(path) as f: | |
| data = json.load(f) | |
| except (OSError, ValueError): | |
| return None | |
| entry = data.get(key) | |
| if isinstance(entry, dict) and isinstance(entry.get("value"), (int, float)): | |
| return float(entry["value"]) | |
| return None | |
| try: | |
| with open(path, encoding="utf-8") as f: | |
| data = json.load(f) | |
| except (OSError, ValueError): | |
| return None | |
| if not isinstance(data, dict): | |
| return None | |
| entry = data.get(key) | |
| if isinstance(entry, dict) and isinstance(entry.get("value"), (int, float)): | |
| return float(entry["value"]) | |
| return None |
Extend plot_wns.py to quantify how well the cts and globalroute worst-slack estimates predict the final WNS. Each design's per-stage error (stage - finish) is normalized by its clock period, parsed from the .sdc, so PDKs with different timing units are comparable. Adds flow/designs/wns_accuracy.png (per-PDK strip plot of normalized estimate error, + optimistic / - pessimistic) and a new flow/designs/README.md with a "## WNS estimate accuracy across PDKs" section: a per-PDK MAE/bias table plus hand-written findings. Covers the 67 designs across 8 PDKs that expose cts/globalroute slack and a parsable clock period; the rest are noted as omitted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
|
@maliberty grt is pretty accurate for the designs in ORFS... so the pathology I saw is not present there... |
1 participant
What
Adds
flow/util/plot_wns.py, a small local script (justpython3, no bazel) that reads the committedrules-base.jsonbaselines and regenerates two things, all committed so they render on GitHub with nothing to run:1. Per-PDK WNS by design —
flow/designs/<pdk>/wns.png+ a## WNSsection inflow/designs/<pdk>/README.md(bar =finishWNS, markers =cts/globalroute). Covers all 9 PDKs with timing baselines (76 designs).2. Cross-PDK WNS estimate accuracy —
flow/designs/wns_accuracy.png+ a## WNS estimate accuracy across PDKssection inflow/designs/README.md. For each design, the per-stage estimate error(stage − finish) / clock_period(clock period parsed from the.sdc) so the PDKs are comparable;+= optimistic,−= pessimistic. 67 designs / 8 PDKs (designs with nocts/globalrouteslack or no parsable period are omitted).Run with:
Regeneration is idempotent: generated content lives between markers, so hand-written findings prose is preserved.
Why
No OpenROAD/ORFS run is needed — the data already lives in the tree, so the plots are deterministic and the committed PNGs + tables render directly on GitHub. The goal is to share the code and the findings in a form that can be viewed and discussed online without running anything.
Findings
WNS estimate accuracy (cross-PDK):
cts→globalroute):sky130hs10.5%→1.9%,gt2n3.3%→0.0%,gf122.2%→1.1%.ihp-sg13g2/gf180are accurate already atcts.ctsis biased optimistic everywhere (bias ≥ 0);globalrouteover-corrects into pessimism forsky130hd(−3.5%),sky130hs,gf12,nangate45.sky130hdis the exception where routing makes the estimate worse (grt MAE 3.5% > cts 2.9%).sky130hs/gcdcts +45.9% (corrected by grt),asap7/swerv_wrapper+14.9% at both stages.This is the design-level companion to the per-net GRT-vs-RCX divergence in
flow/docs/rcx(#4302).asap7 WNS by design:
All 18 asap7 designs close with negative setup slack;
swerv_wrapper/mock-aluare ~10× the −15…−50 cluster.Notes
flow/util/requirements_lock.txt) + stdlib.rules-base.json(e.g. asap7minimal,rcx-fanout-*) are skipped; finish-only designs (e.g.gf55/aes) appear in the per-PDK view but not the accuracy view.🤖 Generated with Claude Code