Repository navigation
docs(readme): include kornia in benchmark tables - #71
Merged
Merged
Conversation
Reviewer's GuideAdds Kornia benchmark results across RGB, 9-channel, and video tables; introduces new scenario overview plots for 9-channel and video benchmarks; and aligns README, figure generation, and benchmark docs with the newly published artifacts and data exports. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="scripts/paper/generate_figures_and_insights.py" line_range="732-738" />
<code_context>
+ return pd.DataFrame()
+
+ rows: list[dict[str, object]] = []
+ for (regime, regime_label, library), group in scenario.groupby(
+ ["regime", "regime_label", "library"],
+ observed=True,
+ sort=False,
+ ):
+ measured = group[group["measured"]]
+ rows.append(
+ {
+ "regime": str(regime),
</code_context>
<issue_to_address>
**issue (bug_risk):** Clarify how multiple rows per (regime_label, library) are handled before taking a single median per facet.
The aggregation is per (regime, regime_label, library), but the plotting later assumes one row per (regime_label, library) and does `facet[facet['library'] == library].iloc[0]`. If multiple `regime` values share a (`regime_label`, `library`), the selected row is arbitrary and could mask variation. Consider aggregating `summary` further to one row per (`regime_label`, `library`) before plotting, or validating that this combination is unique and failing otherwise.
</issue_to_address>
### Comment 2
<location path="scripts/paper/generate_figures_and_insights.py" line_range="749-753" />
<code_context>
+ },
+ )
+
+ summary = pd.DataFrame(rows)
+ summary["regime_label"] = pd.Categorical(summary["regime_label"], categories=REGIME_ORDER, ordered=True)
+ summary["library"] = pd.Categorical(summary["library"], categories=LIBRARY_ORDER, ordered=True)
+ summary = summary.sort_values(["regime_label", "library"]).reset_index(drop=True)
+ summary.to_csv(GENERATED / f"figure_{output_stem}_overview.csv", index=False)
+
</code_context>
<issue_to_address>
**suggestion:** Using fixed Categorical categories may drop or misorder unexpected regime/library labels.
Coercing `regime_label` and `library` to Categoricals with fixed `REGIME_ORDER`/`LIBRARY_ORDER` will turn any new/unknown labels into `NaN`, which then get sorted to the end. This can silently misorder new regimes/libraries and change their dtype in the CSV in a surprising way. You could either (a) validate that all labels are in the known order lists and fail if not, or (b) extend the category order at runtime by appending any unknown labels so they remain valid, deterministically ordered categories.
```suggestion
summary = pd.DataFrame(rows)
# Build category orders that preserve the configured order for known labels
# while appending any previously unseen labels in a deterministic (sorted) order.
regime_labels = summary["regime_label"].astype(str)
library_labels = summary["library"].astype(str)
summary["regime_label"] = regime_labels
summary["library"] = library_labels
regime_seen = set(regime_labels.unique())
library_seen = set(library_labels.unique())
unknown_regimes = sorted(regime_seen - set(REGIME_ORDER))
unknown_libraries = sorted(library_seen - set(LIBRARY_ORDER))
regime_categories = [c for c in REGIME_ORDER if c in regime_seen] + unknown_regimes
library_categories = [c for c in LIBRARY_ORDER if c in library_seen] + unknown_libraries
summary["regime_label"] = pd.Categorical(
summary["regime_label"],
categories=regime_categories,
ordered=True,
)
summary["library"] = pd.Categorical(
summary["library"],
categories=library_categories,
ordered=True,
)
summary = summary.sort_values(["regime_label", "library"]).reset_index(drop=True)
summary.to_csv(GENERATED / f"figure_{output_stem}_overview.csv", index=False)
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates benchmark documentation and generated artifacts to include Kornia across RGB, 9-channel, and video scenarios, plus adds new overview plots for the 9-channel and video benchmarks.
Changes:
- Add 9-channel and video “scenario overview” plot generation and publish their CSV summaries.
- Update README benchmark tables to include Kornia (and additional libraries where applicable) across scenarios.
- Refresh documentation to match published result artifacts and new figure lineup/numbering.
Reviewed changes
Copilot reviewed 7 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/paper/generate_figures_and_insights.py | Adds scenario overview figure/CSV generation and updates figure metadata + sync lists. |
| docs/good_plots.md | Updates the recommended figure inventory to include new overview plots and adjust roles. |
| docs/benchmark_scope.md | Refreshes status table entries to point at published artifacts and updated outcomes. |
| docs/benchmark_methodology.md | Documents that README figures are generated from the same CSVs and adds overview-plot context. |
| docs/benchmark_data/figure_video16f_overview.csv | Adds published video overview summary data. |
| docs/benchmark_data/figure_image9ch_overview.csv | Adds published 9-channel overview summary data. |
| README.md | Inserts new overview figures and expands benchmark tables with Kornia (and others) columns. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: 2693a233bd
ℹ️ 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".
Use regime-wide transform denominators in scenario overview summaries and plots, validate known labels before categorical sorting, and preserve manual layout when saving overview figures. Update generated overview CSVs, figures, README caption, and regression tests.
ternaus
force-pushed
the
codex/kornia-readme-plots
branch
from
May 18, 2026 05:55
99fb912 to
1784d1d
Compare
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.
Add Kornia columns for RGB, 9-channel, and video README tables, generate 9-channel and video overview plots, and align docs with the published artifacts.
Summary by Sourcery
Update benchmark documentation and figure generation to incorporate Kornia results and new scenario overview plots for 9-channel images and video.
Enhancements:
Documentation:
Chores: