Repository navigation
Conversation
Block tensors reach the whole-buffer CUDA path only when both operands share one packed layout. Every other case (permute/repartition materialization, add/axpy with a key superset, truncation, GEMM pack and scatter, SVD sector assembly) loops over blocks on the host with one launch and one ledger round per block. The note proposes a symmetry-agnostic dense primitive: one elementwise operation over a list of differently shaped strided view tuples in one node, scheduled by load-balanced element chunks rather than per block, with plan-time segment classes for permuted copies and structure-keyed plan caching. Open questions are listed for maintainer decision. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review Summary: ❌ FailedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new active design document, segmented_strided_elementwise.md, detailing a dense primitive for segmented strided elementwise execution on CUDA to optimize block-sparse operations into a single kernel launch. It also updates the CUDA backend README and block coalescing documentation to reference this new design. There are no review comments, and I have no feedback to provide.
|
Review Bot: review of exact head No defect found. What the PR does. Doc-only. Adds a design note for one-launch segmented strided elementwise execution on CUDA (per-segment descriptors, load-balanced chunk schedule, segment classes, plan caching) and links it from the CUDA docs index and What I reviewed. All three changed files. I checked every relative link in the new note for existence. I compared its claims about the current code with Verified. All nine relative links resolve (including Not checked. The Cytnx#1229 claim and the Review tier: light (documentation/data-only new pull request (3 files)) · model: claude-sonnet-5-5 · effort: medium |
The first draft treated each operand as one buffer with one access. Packed CUDA storage keeps one completion ledger per block, and Stream::wait_on does not deduplicate, so the draft would have kept one cudaStreamWaitEvent per block. The submission now acquires only the touched children, waits once per distinct completion, and publishes one shared completion. Other changes: - List the per-block callers that exist today, including CUDA block-SVD materialization (one launch per selected singular vector) and the contraction zero fill. Coalesced-GEMM pack and scatter are future callers only. - Build segments from block view mappings rather than storage offsets. Fuse adjacent contiguous segments, so the unpadded same-layout case becomes the allocation-wide path. - Express sorted selections as runs of segments rather than index tables. - Keep 64-bit allocation-relative bases with a per-segment 32-bit decode. - Replace the per-segment barrier walk with a load-balancing search that uses one barrier per chunk. - Use one segment list per operation form instead of emulating missing inputs with zero, which would change signed zeros and non-finite scaling. - Add batched-copy, CUB and graph-capture alternatives, and give recommendations on the open questions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The no-affinity rule assumes a producer's stream is only available once the producer has completed. Programmatic dependent launch (compute capability 9.0+) is the case where overlapping with a running producer has value, so record its three forms and how each fits the runtime. Only same-stream use within one submission and programmatic graph edges fit without changes. Cross-stream programmatic events are undocumented outside stream capture, and a misregistered dependency would be a silent race, so they need a hardware test before use. Also note same-stream PDL for the launches of one segmented submission. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Doc-only. Adds
docs/backends/cuda/segmented_strided_elementwise.mdand links it from the CUDA docs index and fromblock_coalescing.md.It also adds a short programmatic dependent launch (PDL) subsection to
docs/backends/cuda/runtime.md. That subsection records PDL as the one case the no-stream-affinity rule does not cover. Same-stream use within one submission and programmatic graph edges fit the runtime. Cross-stream programmatic events are undocumented outside stream capture and need a test on compute capability 9.0 or later before use.Problem. Block tensors use the allocation-wide
cuda_partitioned_*path only when both operands share one packed layout, and never forpermute/repartitionviews or three-operandadd. Everything else makes one CUDA submission per block: assignment from views, key-supersetadd/axpy, zero fills (including the contraction's unbound-block fill), CUDA block-SVD sector assembly, and block-SVD factor materialization, which launches once per selected singular vector. Packed CUDA storage also keeps one completion ledger per block, andStream::wait_ondoes not deduplicate. After an allocation-wide operation, the next whole-tensor acquisition therefore issues onecudaStreamWaitEventper block on the same event.Proposal. A symmetry-agnostic dense primitive: one elementwise operation over a list of differently shaped strided view tuples, in one submission.
make_multi_iteration_plan. Adjacent contiguous segments fuse, so the unpadded same-layout case is the allocation-wide path, and padding is never touched.transform_lbs) uses one barrier per chunk, however many tiny blocks the chunk crosses.add) gets its own segment list. Missing inputs are not emulated with zero, which would turn-0.0into+0.0and give NaN for non-finite scaling factors.cudaMemcpyBatchAsync/cudaMemcpy3DBatchAsync, CUBDeviceMemcpy::Batched, and CUDA graph capture of the per-block loop.Open questions for maintainer decision, with recommendations in the note: separate CUDA block storage (recommend declining), when to add the tiled transpose (after benchmarks), plan ownership once caching is added, whether to merge operation forms into one launch, and whether to support async block tensors.
Verification. I checked the note's claims against
block_tensor_linear.hpp,block_tensor_svd.hpp,block_tensor_contract.hpp,two_site_split.hpp,backend/cuda/buffer.hpp,runtime.cpp,cuda_storage.hppand the CUDA elementwise plan and kernel. I checked Cytnx#1229 withgh, and the CUDA 12.8 batch-copy and CUDA 12.1 kernel-parameter facts against NVIDIA's documentation.python3 scripts/check-docs.pyandpython3 -m unittest discover -s tests/docspass. I did not run the Doxygen web build. No code changed, and no GPU measurement backs the performance statements.🤖 Generated with Claude Code