Skip to content

Modify tile size for DMA operations to use Y_SIZE#1319

Open
wypku wants to merge 1 commit intoaws-neuron:masterfrom
wypku:patch-1
Open

Modify tile size for DMA operations to use Y_SIZE#1319
wypku wants to merge 1 commit intoaws-neuron:masterfrom
wypku:patch-1

Conversation

@wypku
Copy link
Copy Markdown

@wypku wypku commented Apr 25, 2026

IMPORTANT! If this is a documentation PR for a specific release, this PR must go the corresponding release branch (release-X.XX.X). If it is an "out-of-band" doc update, the PR must go to the master branch.

Required PR information

To expedite approvals and merges for releases, provide the following information (select the ... button to the right at the top of your PR message to edit it):

AWS email alias: ianww@amazon.com [feel free to contact!]

Description: {This is a bug in the code, the dimension should be (128, 512)}

Date this must be published by: {better asap}

Link to ReadTheDocs staging for this branch's doc changes: https://awsdocs-neuron.readthedocs-hosted.com/en/latest/nki/get-started/about/tiling-overview.html

Set the docs-review-needed label on the PR for tracking. No permission on my side to set

Before you request approvals

Run a spelling and grammar check over your prose and make the changes it suggests. VSCode has a number of extensions (cSpell, LTeX) that you can use. You can also provide the rendered HTML for (or a cut-and-paste of) your pages to an AI and have it correct your spelling, grammar, and formatting issues. If you need an advanced prompt, contact @erickson-doug.

Approvers

We require 3-4 approvers to merge for non-trivial content changes (where a "trivial" change is a typo/grammar fix or a minor update to the format syntax):

  1. A senior+ engineer who will review your documentation for technical accuracy and clarity in communicating the technical concepts in your work
  2. A product manager for your Neuron component area who will review it for customer relevance and product/component/feature messaging
  3. The lead tech writer (@erickson-doug) who will review your work for overall doc design and quality, and perform the merge when all approvals are met
  4. (For PRs with code/notebook submissions) A QA/test engineer who can run your code and confirm the results.

Make sure you get a commitment from these reviewers in advance! It's hard to get good quality doc reviews in order in the 11th hour of a release.

Note: For trivial changes, you only need @erickson-doug's approval. He will merge your content once he's confirmed the fixes on staging.

Doc review checklist

Engineering reviewer checklist

  • I've confirmed that the contributions in this PR meet the current AWS Neuron writing guidelines.
  • I've confirmed that the documentation submitted is technically correct to the best of my knowledge.
  • I've confirmed that the documentation submitted has no spelling or grammar errors or use of internal jargon/terminology.
  • I've verified the changes render correctly on RTD (link above).
  • (If code is included) I've run tests to verify the contents of the change.

For PRs that include code or notebook examples

MANDATORY: PR must include test run output

Provide this information for the QA reviewer in order to expedite their review.

Test run output:
Specify the release version, instance size and type, OS type and test output.

For Training tutorials:

{Convergence graph for training tutorials}

{Performance metrics average_throughput, latency_p50, latency_p99 and MFU% if available}

Make sure this PR contains correct classification terms (Alpha, Beta, and Stable).

If possible, provide your results or a link to them for the reviewer to check your work.

Code example/notebook content PR checklist

  • (If applicable) I've automated a test to safeguard my changes from regression.
  • (If applicable) I've posted test collateral to prove my change was effective and not harmful.
  • (If applicable) I've added someone from QA to the list of reviewers. Do this if you didn't make an automated test or feel it's appropriate for another reason.
  • (If applicable) I've reviewed the licenses of updated and new binaries and their dependencies to make sure all licenses are on the pre-approved Amazon license list. See https://inside.amazon.com/en/services/legal/us/OpenSource/Pages/BlessedOpenSourceLicenses.aspx.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the tiling overview documentation example to size SBUF tiles and DMA slices based on Y_SIZE (512) instead of a hard-coded 256, matching the stated expected tensor shape (128, 512).

Changes:

  • Update in_tile/out_tile allocations to use (P_DIM, Y_SIZE).
  • Update DMA copy source/destination slicing to copy 0:Y_SIZE instead of 0:256.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +129 to +131
nki.isa.dma_copy(dst=in_tile, src=in_tensor[0:P_DIM, 0:Y_SIZE])
nki.isa.reciprocal(dst=out_tile, data=in_tile)
nki.isa.dma_copy(dst=out_tensor[0:P_DIM, 0:256], src=out_tile)
nki.isa.dma_copy(dst=out_tensor[0:P_DIM, 0:Y_SIZE], src=out_tile)
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

The surrounding prose/output describe an exponential example (e.g., output values ~2.7188), but this code calls nki.isa.reciprocal. Please align the example so the operation matches the narrative/output (either switch the ISA op to an exponential variant, or update the text/output to describe reciprocal).

Copilot uses AI. Check for mistakes.

# Process first tile
nki.isa.dma_copy(dst=in_tile, src=in_tensor[0:P_DIM, 0:256])
nki.isa.dma_copy(dst=in_tile, src=in_tensor[0:P_DIM, 0:Y_SIZE])
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

This snippet imports nki.isa as nisa earlier in the same code block, but uses nki.isa.* here. To avoid confusing readers (and an otherwise-unused alias), either use the nisa alias consistently in these calls or change/remove the alias import.

Copilot uses AI. Check for mistakes.
@aws-mgld aws-mgld self-requested a review May 4, 2026 20:29
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