Repository navigation
Fix duplicated performance history and missing Nelux benchmarks - #3
Open
NevermindNilas wants to merge 8 commits into
Open
NevermindNilas wants to merge 8 commits into
NevermindNilas wants to merge 8 commits into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The Validate workflow likely cannot gh run download without granting actions: read, and a couple of robustness issues (env var handling / ffmpeg version probing) should be fixed before merging.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 3
Open (3)
What changed in this PR
This PR hardens benchmark publication and reporting by making runner/hardware grouping stable over time, ensuring Nelux installs from the correct release wheel for the active PyTorch ABI, and making failures explicit in charts/tables instead of silently disappearing.
Changes:
- Add validation that blocks publishing stale results, missing charts, or incomplete/failed Nelux runs.
- Refactor reporting to include failed decoders explicitly (tables + bar charts) and update history trending to group hosted runners into stable cohorts with gaps for failures.
- Update GitHub Actions workflows to use a shared setup action, enforce publication rules, and support chart-only re-renders.
| File | Description |
|---|---|
tests/test_validate_results.py |
Adds tests for publication gating (stale run rejection, missing chart, failed Nelux). |
tests/test_reporting.py |
Verifies failed decoders appear in tables/charts without fake FPS, and that chart failures propagate. |
tests/test_install_nelux.py |
Tests wheel selection across Python/platform/torch ABI compatibility. |
tests/test_history.py |
Tests rerun replacement semantics, “gap not zero” behavior, and hosted-run cohort grouping. |
src/validate_results.py |
Implements pre-publication validation for run identity, Nelux completeness, history consistency, and chart presence. |
src/reporting.py |
Centralizes bar chart + markdown generation with explicit handling of unavailable decoders. |
src/install_nelux.py |
Installs latest Nelux release asset matching Python tags and torch ABI; verifies import. |
src/history.py |
Persists richer metadata and plots trends by stable cohorts with gaps for failures/unavailability. |
src/backends/celuxdecoder.py |
Improves Nelux import diagnostics and switches Nelux decode to CPU NumPy backend. |
requirements.txt |
Adds packaging for wheel/tag parsing used by Nelux installer. |
README.md |
Updates methodology/history documentation and points example chart to latest release asset. |
main.py |
Adds package/version + input checksum metadata, frame-count validation, and uses refactored reporting helpers. |
.github/workflows/Weekly-Benchmark.yaml |
Reworks scheduled/dispatch benchmark workflow: shared setup, tests, validation, artifact upload, guarded publish. |
.github/workflows/Validate.yaml |
Adds CI workflow for reporting tests, historical chart regeneration, and optional chart replay from a prior run. |
.github/actions/setup-benchmark/action.yml |
Introduces composite action to install FFmpeg runtime, Python deps, and a matching Nelux wheel. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+14
to
+16
| permissions: | ||
| contents: read | ||
|
|
Comment on lines
+332
to
+334
| ffmpegVersion = subprocess.check_output(["ffmpeg", "-version"], text=True).splitlines()[0] | ||
| for name, data in decodingResults.items(): | ||
| data["version"] = packages[decoderPackages[name]] if name in decoderPackages else ffmpegVersion |
Comment on lines
+15
to
+25
| if result["runnerInfo"].get("runId") != os.environ["GITHUB_RUN_ID"]: | ||
| raise ValueError(f"{resolution}: results are not from this Actions run") | ||
| nelux = result["decoders"]["Nelux"] | ||
| if "error" in nelux or nelux.get("successfulRuns") != result["config"]["runs"] or nelux.get("fps", 0) <= 0: | ||
| raise ValueError(f"{resolution}: Nelux must succeed in every timed iteration: {nelux}") | ||
| if not info.get("frameCount") or nelux["frameCount"] != info["frameCount"]: | ||
| raise ValueError(f"{resolution}: incomplete Nelux decode") | ||
| history = json.loads((directory / "history" / f"{resolution}_history.json").read_text()) | ||
| record = history["runs"][-1] | ||
| if record.get("runId") != os.environ["GITHUB_RUN_ID"] or record["decoders"]["Nelux"]["fps"] != nelux["fps"]: | ||
| raise ValueError(f"{resolution}: history does not match current measurements") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The biweekly history plot used ephemeral GitHub Actions runner names as separate hardware groups, producing a new single-date panel for every run. Nelux was installed from an obsolete 0.9.2 wheel that fails against current PyTorch with an undefined native symbol; the import error was swallowed and its chart entry disappeared.
This change groups old hosted measurements into one labelled legacy series, uses stable runner pools and CPU cohorts for new results, and preserves failures as gaps. Nelux now installs from the latest release asset matching Python, platform and PyTorch ABI, verifies its import, and decodes RGB24 NumPy frames on CPU. Charts and release tables include unavailable decoders explicitly.
Results now retain package versions, CPU details, input checksums and Actions run identity. Publication rejects stale outputs, missing charts and incomplete Nelux runs. Retries replace the same history record; distinct runs get distinct release tags. Branch benchmark runs upload artifacts without publishing or committing history. The two checked-in historical charts were regenerated on GitHub Actions; original historical JSON and prior releases are unchanged.
Validation (all execution on GitHub Actions; no local benchmarks or Python scripts):
Hosted runners can vary even within a CPU cohort. Old runs lack hardware metadata and remain explicitly separated from the new cohorts; these trends should not be treated as controlled performance regression tests.
Known existing backend failures remain explicit: TorchAudio lacks its removed io API, VideoReaderRS fails to import, and ImageIO reports 301 frames for the 300-frame clips. The new frame-count check excludes the invalid ImageIO timing rather than ranking it as a valid decode.