Skip to content

Cherrypick envs throughput benchmarking results - #1240

Merged
xyao-nv merged 2 commits into
release/0.3.0from
xyao/docs/backport-pr-1234
Sep 10, 2026
Merged

xyao-nv merged 2 commits into
release/0.3.0from
xyao/docs/backport-pr-1234

Conversation

@xyao-nv

@xyao-nv xyao-nv commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Cherrypick envs throughput benchmarking results #1234

Signed-off-by: Clemens Volk <cvolk@nvidia.com>
(cherry picked from commit 442eae3)
Signed-off-by: Clemens Volk <cvolk@nvidia.com>
(cherry picked from commit f196bb2)
@xyao-nv
xyao-nv marked this pull request as ready for review September 10, 2026 17:16
@greptile-apps

greptile-apps Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

The PR appears safe to merge after improving the non-blocking single-GPU benchmark reproduction reference.

Findings

  1. P2 Benchmark Configuration Mismatch

Summary

  • Documents measured throughput across 1–1,024 parallel environments.
  • Documents OSMO scaling across 1, 2, 4, and 8 concurrent GPUs.
  • Adds benchmark scope, hardware details, tested revisions, and navigation links.
  • The single-GPU reproduction link points to a provisional configuration that does not match the published run parameters.

Reviews (1) · Last reviewed commit: "Refine performance scaling wording"

Comment thread docs/pages/references/performance.rst
Comment thread docs/pages/references/performance.rst
Comment thread docs/pages/references/performance.rst
@arena-review-bot

Copy link
Copy Markdown
Contributor

🤖 Isaac Lab-Arena Review Bot

Summary

Docs-only backport of #1234 (merged to main earlier today) onto release/0.3.0: a Performance and scaling reference page, two SVG charts, and two sentences on the landing page. I diffed all four files against main — they are byte-identical, so the cherry-pick is faithful and the only index.rst difference between the branches is the pre-existing development-version note. I re-checked the arithmetic independently (throughputs against the mean step times, the 2.02x/4.05x/7.95x speedups, the 20:55 → 2:38 conversion, 32 Runs, the sub-2% per-Run spread): all consistent.

Design, Boundaries & Scope

The one thing this backport changes versus the original is the permanence of the reproducibility gap. Tested revisions pins b0cd0b38e and 4ee056866, which exist only on the still-open PRs #1151 and #1188 — not on main, not on release/0.3.0 — and experiment_configs/perflab/ is on neither branch. On main that fixes itself when those PRs land; release/0.3.0 only takes cherry-picks, so a 0.3.0 reader will never be able to reproduce these numbers from the released tree, and the permalinks break if either branch is squash-merged and deleted. Worth deciding whether the configs should ride along with this backport.

Findings

🟡 Warning: docs/pages/references/performance.rst:151 — pinned revisions and benchmark configs are unreachable from release/0.3.0; see above.

🔵 Improvement: docs/pages/references/performance.rst:21 — "collected for this release" reads as 0.3.0 on this branch, but the measured revision has diverged from it.

The findings from the original review on #1234 carry over unchanged, since the content is identical — most relevantly that the linked camera_free_benchmark_experiment.yaml still sets num_envs: 1 / num_steps: 10 rather than the documented 300-step sweep, and that the landing-page sentence at docs/index.rst:443 sits directly under the "thousands of heterogeneous (object-level) environments" claim while the benchmark put the same rubiks_cube_hot3d_robolab in all 1,024 environments. Not re-posting them inline here; they were visible when #1234 merged.

Test Coverage

Documentation only — no code paths, so no unit or simulation tests apply. Docs build and pre-commit both pass on this branch.

Verdict

Minor fixes needed

@xyao-nv
xyao-nv merged commit 117c3da into release/0.3.0 Sep 10, 2026
11 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.

3 participants