Skip to content

[places] Guard empty tiled mdspan shape size#9149

Open
fallintoplace wants to merge 2 commits into
NVIDIA:mainfrom
fallintoplace:fix/tiled-mdspan-empty-size
Open

[places] Guard empty tiled mdspan shape size#9149
fallintoplace wants to merge 2 commits into
NVIDIA:mainfrom
fallintoplace:fix/tiled-mdspan-empty-size

Conversation

@fallintoplace
Copy link
Copy Markdown

What changed

  • return 0 early from tiled_mdspan_shape::size() when the original shape is empty
  • add a regression test covering the empty-shape case

Why

tiled_mdspan_shape::size() computes tile_per_part and then uses (tile_per_part - 1) in unsigned arithmetic.

For empty input, original_shape.size() is 0, so ntiles == 0 and tile_per_part == 0. That makes (tile_per_part - 1) underflow and can turn an empty shape into a huge nonzero size.

Impact

Empty tiled mdspan shapes now report 0 elements instead of underflowing.

Validation

  • added a focused header unittest for the empty-shape case
  • attempted cmake --preset cudax-cpp17, but validation is blocked in this environment because nvcc / CUDAToolkit is not available

@copy-pr-bot
Copy link
Copy Markdown
Contributor

copy-pr-bot Bot commented May 27, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@cccl-authenticator-app cccl-authenticator-app Bot moved this from Todo to In Progress in CCCL May 27, 2026
@fallintoplace fallintoplace marked this pull request as ready for review May 27, 2026 23:29
@fallintoplace fallintoplace requested a review from a team as a code owner May 27, 2026 23:29
@fallintoplace fallintoplace requested a review from andralex May 27, 2026 23:29
@cccl-authenticator-app cccl-authenticator-app Bot moved this from In Progress to In Review in CCCL May 27, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 27, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5be93582-1bb9-45cd-bd0e-f44a1e0af5ab

📥 Commits

Reviewing files that changed from the base of the PR and between 1e2d0c6 and 8e5f3ad.

📒 Files selected for processing (1)
  • cudax/include/cuda/experimental/__places/partitions/tiled_partition.cuh
🚧 Files skipped from review as they are similar to previous changes (1)
  • cudax/include/cuda/experimental/__places/partitions/tiled_partition.cuh

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Tiling partition now correctly reports a size of zero for empty (zero-length) inputs, preventing unnecessary tiling computations.
  • Tests

    • Added a unit test to verify empty-input handling for tiled partition sizing.

important:

Walkthrough

tiled_mdspan_shape::size() adds an early-return guard returning 0 when the underlying 1D shape has zero elements, avoiding tiling math for empty inputs. A unit test validates this behavior, confirming that partitioned empty shapes correctly report size 0.

Changes

Empty tiled partition size handling

Layer / File(s) Summary
Early return for empty shapes and test
cudax/include/cuda/experimental/__places/partitions/tiled_partition.cuh
tiled_mdspan_shape::size() returns 0 immediately when input shape size is zero, bypassing tile calculation. Unit test constructs a mocked zero-element shape and verifies the method returns 0 across tile_size=4, part_id=0, nparts=3 configurations.

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

@miscco
Copy link
Copy Markdown
Contributor

miscco commented May 28, 2026

/ok to test 1e2d0c6

@github-actions
Copy link
Copy Markdown
Contributor

😬 CI Workflow Results

🟥 Finished in 1h 03m: Pass: 98%/55 | Total: 1d 00h | Max: 1h 02m | Hits: 12%/168641

See results here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

2 participants