Skip to content

Improve Lyapunov benchmark performance reporting - #17

Merged
jamestjsp merged 4 commits into
mainfrom
codex/bb04ad-performance-report
May 11, 2026
Merged

Improve Lyapunov benchmark performance reporting#17
jamestjsp merged 4 commits into
mainfrom
codex/bb04ad-performance-report

Conversation

@jamestjsp

Copy link
Copy Markdown
Owner

Summary

  • optimized BB04AD examples 4.1/4.2 scaling loops by replacing repeated pow() calls with recurrence-based scaling
  • added a benchmark report section for rows where Fortran is faster, sorted by largest advantage
  • completed and committed the ergo performance epic state

Validation

  • uv pip install .
  • .venv/bin/pytest tests/python/test_bb04ad.py -v
  • .venv/bin/pytest tests/python/test_benchmark_report.py -v
  • .venv/bin/pytest tests/python/test_bb03ad.py tests/python/test_bb04ad.py -v
  • .venv/bin/meson setup build-bench-c -Dbuild_benchmarks=true --wipe
  • .venv/bin/meson compile -C build-bench-c
  • .venv/bin/python scripts/benchmark_c_vs_fortran.py --c-build-dir build-bench-c --output benchmark_c_vs_fortran_report.md -r bb03ad bb04ad

Final generated report showed no BB03AD/BB04AD row more than 1.5x slower than Fortran; remaining Fortran-faster rows were BB04AD ex4.4 at 1.11x, 1.07x, and 1.06x.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 651278524f

ℹ️ 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".

lines.append("| Routine | Dataset | N | C11 (μs) | F77 (μs) | F77/C11 Ratio | Delta (μs) |")
lines.append("|---------|---------|---|----------|----------|---------------|------------|")
for c in fortran_faster:
ratio = c.c_mean_us / c.f_mean_us if c.f_mean_us > 0 else float("inf")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Fix inverted ratio reported as F77/C11

The new Fortran Faster Rows table labels the column as F77/C11 Ratio, but the value is computed as c_mean_us / f_mean_us (C11/F77). This inverts the meaning of every reported ratio and can mislead performance decisions (for example, C11=40 and F77=20 is shown as 2.00x even though F77/C11 is 0.50x). Please either compute f_mean_us / c_mean_us or relabel the column to match the current formula.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1594fe1 by relabeling the column to F77 Advantage, matching the existing C11-time / F77-time formula and sort order. The focused report-rendering test was updated and passes.

@jamestjsp
jamestjsp merged commit a3f6c5f into main May 11, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant