Add checkQorMetrics.py inline check script - #4458
Conversation
![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 new script, checkQorMetrics.py, designed to validate local flow runs against QoR dashboard rules by posting numeric metrics to an API. The feedback focuses on improving the script's robustness and portability. Key recommendations include wrapping check_run and fmt_value in try-except blocks to handle unexpected API responses or non-numeric values gracefully, and explicitly specifying encoding='utf-8' when reading and writing files to prevent platform-dependent encoding issues.
🔍 QoR checkMetrics reflect the PR merge build — i.e. what will land on the target branch. Advisory — results are log-only and do not affect build status. The authoritative QoR gate remains the local rules-file check. Commit 62 design(s) checked — 0 with regression(s), 1 without a comparable baseline.
|
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Needs black format |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b34bf90866
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not runs: | ||
| for line in header: | ||
| print(line) | ||
| print( | ||
| f"[WARN] No flow run with a {METADATA_FILENAME} " | ||
| f"found under {reports_dir}." | ||
| ) | ||
| return 0 |
There was a problem hiding this comment.
When a previous invocation has created reports/inline-check.log and a later invocation finds no runs—for example after cleaning metadata or when a --platform/--design filter matches nothing—this early return leaves the old report intact. Anyone reading the documented log afterward sees stale failures or passes from the earlier sweep rather than the current “nothing checked” result; overwrite or remove the log before returning.
Useful? React with 👍 / 👎.
Sweeps flow/reports for metadata.json files, POSTs metrics to the QoR
dashboard's /compare/check/inline endpoint, and reports which rules a
local change would fail against the latest baseline build. Always
exits 0 since it reports rather than gates.
Signed-off-by: Miguel Dalberto Pedro <miguel.pedro@precisioninno.com>
Send JobName so the local baseline resolves to the right pipeline
Empty PipelineId/BranchName don't leave the baseline "unscoped" as
assumed -- the dashboard's PipelineClassification.Classify("") falls
through to null just like Classify(null), and
ResolveLocalBaselineAsync then filters on PipelineName == null,
matching whatever stale/legacy master build has no pipeline recorded
instead of the real latest master build.
JobName (the canonical Jenkins job-folder name, e.g.
jenkins-ci's utilGetJobFolderName()) takes priority over that
classification and scopes the baseline to the actual pipeline.
Default it to OpenROAD-flow-scripts-Public, the pipeline that
uploads master's dashboard metrics, and let --job-name/
$DASHBOARD_JOB_NAME override it for secure/private branches.
Verified end to end: re-running against the same real Jenkins
artifact that previously reported a bogus FAIL against baseline
build 4 now resolves to the real latest master build and reports
PASS with 0% deltas, matching the actual CI result.
Signed-off-by: Miguel Dalberto Pedro <miguel.pedro@precisioninno.com>
Fix the usage docs and narrow the command-line surface
The docstring claimed a plain `make` leaves a metadata.json behind. It
does not: the default goal stops at `finish`, and only `make metadata`
runs genMetrics.py, so a developer following it found nothing to check.
Drop --reports and --log. The sibling report scripts (genReport.py,
genReportTable.py, uploadMetadata.py) all chdir to flow/ and walk a
literal "reports"; making the root configurable here only let the sweep
diverge from what ingestion sees, and needed an INVOCATION_DIR dance to
keep relative paths meaning what the user typed. The log path is now
fixed and reported as an absolute path -- it was printed relative to
flow/, naming a path that does not exist from where the script was run.
Drop --target/--api-url for a --beta flag over two fixed https URLs. An
arbitrary --api-url could send DASHBOARD_API_KEY over a plaintext hop;
with both URLs baked in, that is structurally impossible rather than
guarded at runtime.
Drop the -v alias: genReport.py reads it as --verbose and genMetrics.py
binds it to the variant, so either meaning surprises half the callers.
Report a run directory with no metadata.json from the caller instead of
from the walk, so the warning respects --platform/--design/--variant and
reaches the log file. A scoped run no longer warns about designs it was
never asked to check.
Retry transient request failures three times with linear backoff. One
blip previously cost a design its entire check across a 61-design sweep.
Non-transient HTTP statuses return at once, since re-sending the same
payload only earns the same answer.
Signed-off-by: Miguel Dalberto Pedro <miguel.pedro@precisioninno.com>
b34bf90 to
89f0280
Compare
3 participants
Summary
flow/util/checkQorMetrics.py, a local inline check: sweepsreports/<platform>/<design>/<variant>/metadata.json, POSTs each run's numeric metrics to the QoR dashboard's/compare/check/inlineendpoint, and reports which rules would fail against the latest baseline build (or a pinned--base-commit).checkMetadata.py: compares against a real baseline build + the dashboard's liverule_configsthresholds instead of the committedrules-<variant>.jsongoldens, so metric drift that moves with master doesn't false-positive.