Skip to content

docs: rewrite README to match wrapper documentation standard#13

Open
sayalibhavsar wants to merge 1 commit into
mainfrom
docs/rewrite-readme
Open

docs: rewrite README to match wrapper documentation standard#13
sayalibhavsar wants to merge 1 commit into
mainfrom
docs/rewrite-readme

Conversation

@sayalibhavsar

Copy link
Copy Markdown

Summary

  • Comprehensive rewrite of the numa_streams-wrapper README modeled after the autohpl-wrapper documentation structure
  • All content derived from numa_streams-wrapper source code — no invented functionality
  • Adds all general_setup options (--debug, --no_pkg_install, --no_system_packages, --no_pip_packages, --run_label, --test_tools_release, --verify_skip, --json_skip)
  • Flags missing features: no results_schema.py, no JSON output, no verify_results call, no OpenMetrics reset file

New sections added

  • Command-Line Options: complete listing of both numa_streams-specific and general_setup options
  • What the Script Does: 8-step workflow covering both numa_streams_run and run_numa_stream scripts
  • Dependencies: per-OS package lists (RHEL, Ubuntu, SLES, Amazon Linux)
  • The NUMA STREAMS Benchmark: four STREAM operations (Copy, Scale, Add, Triad), MB/s metric, array sizes, thread scaling
  • Output Files: file listing with descriptions
  • Examples: 10 usage examples covering common scenarios
  • How Cache Sizing Works: base cache detection, top-level cache count, ARM Neoverse SLC handling, thread progression, array sizes
  • Return Codes: error code reference with parameter validation details
  • Notes: architecture support (x86_64 + aarch64/Neoverse), benchmark source (AMD STREAM Dynamic from Phoronix), compiler optimization, missing features, troubleshooting

Test plan

  • Verify all documented options match numa_streams_run source code
  • Verify all documented options match general_setup source code
  • Check package names against numa_streams.json for all OS variants
  • Verify thread scaling logic (powers of 2 up to ncpus)
  • Verify array sizes (512, 1000, 2000, 4000 MiB) match run_numa_stream
  • Confirm ARM Neoverse SLC fallback (32 MiB * numa_nodes) matches source
  • Review examples for correctness

Solves #12
Relates to JIRA: RPOPC-937

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@sayalibhavsar, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 3 minutes and 12 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 93393bc3-a1a6-43ea-88c9-4f694028b430

📥 Commits

Reviewing files that changed from the base of the PR and between a22e8f8 and c48de90.

📒 Files selected for processing (1)
  • README.md
📝 Walkthrough

Walkthrough

The README.md was replaced from a two-line stub into a full 322-line documentation page for the numa_streams benchmark wrapper, covering workflow, command-line options, OS dependencies, output files, cache-sizing behavior, architecture notes (x86_64 vs aarch64/Neoverse), return codes, limitations, and usage examples.

Changes

numa_streams README Expansion

Layer / File(s) Summary
Full README rewrite
README.md
Replaces the minimal stub with complete documentation: wrapper workflow (repo setup, dependency install, cache topology detection, benchmark build, thread/array-size sweep, CSV processing, archiving), command-line flags (NUMA STREAMS and shared test_tools options), OS package requirements, run instructions, usage examples, output file descriptions, cache-sizing methodology, return codes, architecture/optimization/memory notes, known limitations, and troubleshooting.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'docs: rewrite README to match wrapper documentation standard' clearly and concisely summarizes the main change: a comprehensive README rewrite following a specific documentation standard.
Description check ✅ Passed The PR description is comprehensive and well-structured, covering the rewrite objectives, new sections added, missing features flagged, and a detailed test plan with checkpoints.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown

This relates to RPOPC-937

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🧹 Nitpick comments (1)
README.md (1)

21-21: ⚡ Quick win

Add language identifier to code fence.

The command-line options section begins with a fenced code block (line 21) that lacks a language specifier. Markdown best practices (and markdownlint) require specifying the language for syntax highlighting.

Proposed fix:

Replace:

22~
NUMA STREAMS Options:


with:

21~

NUMA STREAMS Options:

This enables syntax highlighting and satisfies markdown linting standards.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` at line 21, The code fence at line 21 in README.md is missing a
language identifier for syntax highlighting. Modify the opening fence from ```
to ```bash to specify the language and comply with markdown linting standards.
This will enable proper syntax highlighting for the NUMA STREAMS Options section
that follows.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@README.md`:
- Around line 277-280: The README.md documentation for the --threads_multiple
parameter validation on line 280 incorrectly states it "silently defaults to 2
if set to 1" when the actual source code in numa_streams_run (lines 449-454)
explicitly exits with error code 1 if the value is less than 2. Update line 280
to change the description from "(silently defaults to 2 if set to 1)" to "(exits
with error if not)" to match the actual validation behavior and align with the
consistent pattern used for documenting --cache_multiply and
--cache_start_factor on the preceding lines.

---

Nitpick comments:
In `@README.md`:
- Line 21: The code fence at line 21 in README.md is missing a language
identifier for syntax highlighting. Modify the opening fence from ``` to ```bash
to specify the language and comply with markdown linting standards. This will
enable proper syntax highlighting for the NUMA STREAMS Options section that
follows.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: fb790010-d72f-4fe1-a474-01042eeb71c7

📥 Commits

Reviewing files that changed from the base of the PR and between 9f82dce and a22e8f8.

📒 Files selected for processing (1)
  • README.md

Comment thread README.md Outdated
@sayalibhavsar sayalibhavsar force-pushed the docs/rewrite-readme branch 3 times, most recently from 65c934e to d49c72f Compare June 15, 2026 11:00
Comprehensive rewrite of the numa_streams-wrapper README modeled after
the autohpl-wrapper documentation structure. All content is derived
from the numa_streams-wrapper source code.

Adds detailed sections for: command-line options (including all
general_setup options), step-by-step workflow covering both
numa_streams_run and run_numa_stream scripts, per-OS dependencies,
STREAM operations and metrics, output files, usage examples, cache
sizing and thread scaling logic, return codes, and notes covering
architecture support, ARM Neoverse SLC handling, missing features
(no schema validation or JSON output), and troubleshooting.
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