Skip to content

Update README.md#1669

Merged
ktangsali merged 1 commit into
mainfrom
coreyjadams-patch-ring-attn-readme
May 22, 2026
Merged

Update README.md#1669
ktangsali merged 1 commit into
mainfrom
coreyjadams-patch-ring-attn-readme

Conversation

@coreyjadams
Copy link
Copy Markdown
Collaborator

PhysicsNeMo Pull Request

Point out that we need matplotlib for the ring attn scaling plots.

Description

Checklist

Dependencies

Review Process

All PRs are reviewed by the PhysicsNeMo team before merging.

Depending on which files are changed, GitHub may automatically assign a maintainer for review.

We are also testing AI-based code review tools (e.g., Greptile), which may add automated comments with a confidence score.
This score reflects the AI’s assessment of merge readiness and is not a qualitative judgment of your work, nor is
it an indication that the PR will be accepted / rejected.

AI-generated feedback should be reviewed critically for usefulness.
You are not required to respond to every AI comment, but they are intended to help both authors and reviewers.
Please react to Greptile comments with 👍 or 👎 to provide feedback on their accuracy.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 22, 2026

Greptile Summary

Adds a documentation note to the ring attention example README clarifying that matplotlib is required to run the scaling plot script.

  • Inserts a > Note! callout before the python plot_scaling_results.py code block, directing users to pip install matplotlib if they haven't already.

Important Files Changed

Filename Overview
examples/minimal/ShardTensorExamples/6_ring_attention/README.md Adds a note that matplotlib must be installed before running the scaling plot script.

Reviews (1): Last reviewed commit: "Update README.md" | Re-trigger Greptile

@ktangsali ktangsali merged commit f14c46d into main May 22, 2026
5 checks passed
@ktangsali
Copy link
Copy Markdown
Collaborator

Ideally this should have gone into RC, but since this is just a readme change, main or Rc does not matter too much.

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.

2 participants