Skip to content

fix(cuda.compute): ignore NumPy field titles in gpu_struct - #11578

Merged
shwina merged 3 commits into
NVIDIA:mainfrom
efegokdemir:codex/issue-11576-gpu-struct-titles
Oct 2, 2026
Merged

shwina merged 3 commits into
NVIDIA:mainfrom
efegokdemir:codex/issue-11576-gpu-struct-titles

Conversation

@efegokdemir

Copy link
Copy Markdown
Contributor

Summary

Fixes #11576. gpu_struct now iterates the canonical names in a structured NumPy dtype instead of every dtype.fields entry, so field titles are treated as aliases rather than extra members.

Changes

  • Build dtype-backed gpu_struct field mappings from dtype.names.
  • Add a regression test covering a titled NumPy field and verifying names, offsets, and itemsize.

Testing

  • python3 -m py_compile python/cuda_cccl/cuda/compute/struct.py python/cuda_cccl/tests/compute/test_struct_numpy_dtype.py — passed.
  • git diff --check — passed.
  • python3 -m pytest python/cuda_cccl/tests/compute/test_struct_numpy_dtype.py -q — not runnable here because pytest and NumPy are not installed.
  • GPU-backed CCCL tests were not run because this macOS environment has no NVIDIA GPU/CUDA toolkit.

Notes

I checked open PRs for #11576 and gpu_struct; no overlapping implementation was open. PR #11470 addresses structured layout validation, but does not fix NumPy field-title alias handling. AI assistance was used to prepare this contribution; the changed code and validation results were reviewed before submission.

@efegokdemir
efegokdemir requested a review from a team as a code owner September 22, 2026 06:20
@efegokdemir
efegokdemir requested a review from shwina September 22, 2026 06:20
@copy-pr-bot

copy-pr-bot Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

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.

@github-project-automation github-project-automation Bot moved this to Todo in CCCL Sep 22, 2026
@cccl-authenticator-app cccl-authenticator-app Bot moved this from Todo to In Review in CCCL Sep 22, 2026
@coderabbitai

coderabbitai Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: NVIDIA/cccl/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b0ff5177-2208-41bd-83cc-dc5d2596dbcb

📥 Commits

Reviewing files that changed from the base of the PR and between 715717c and fb4c8b4.

📒 Files selected for processing (1)
  • python/cuda_cccl/tests/compute/test_struct_conversions.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.


📝 Summary

Summary by CodeRabbit

  • Refactor
    • Simplified internal NumPy dtype conversion; behavior is unchanged.
  • Tests
    • Added coverage confirming that NumPy field titles do not appear as additional structure fields.

Walkthrough

The NumPy dtype field mapping expression is reformatted without changing its behavior. A regression test checks that a titled field does not appear as an additional gpu_struct field and verifies the resulting names, offsets, and item size.

Changes

NumPy field title handling

Layer / File(s) Summary
Canonical field extraction and layout validation
python/cuda_cccl/cuda/compute/struct.py, python/cuda_cccl/tests/compute/test_struct_conversions.py
The dtype field mapping expression is reformatted. The new test checks that a titled field is not an additional gpu_struct field and verifies the field names, offsets, and item size.

Assessment against linked issues

Objective Addressed Explanation
[ #11576 ] Do not treat NumPy field titles as additional gpu_struct fields; preserve the expected names, offsets, and item size. ✅

Suggested reviewers: naderalawar

Priority: ➖ Normal

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to fb4c8

NumPy field titles are treated as aliases, and the added regression checks the resulting layout. No concrete merge-blocking behavior risk is apparent.


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

@HelloWorldU HelloWorldU left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we may need to wait for a maintainer to confirm this

@shwina shwina left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you so much!

@shwina

shwina commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

/ok to test c235df6

@HelloWorldU

Copy link
Copy Markdown
Contributor

@shwina l've noticed you approved this PR, could you please also take a look about this #11470?

@shwina

shwina commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

I believe all that is needed here is a rebase against main.

@shwina

shwina commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

@shwina l've noticed you approved this PR, could you please also take a look about this #11470?

@HelloWorldU thanks - yes, I will take a look

@shwina
shwina force-pushed the codex/issue-11576-gpu-struct-titles branch from c235df6 to 715717c Compare September 28, 2026 15:11
@shwina

shwina commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

/ok to test

@copy-pr-bot

copy-pr-bot Bot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

/ok to test

@shwina, there was an error processing your request: E1

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/1/

@shwina

shwina commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

/ok to test 715717c

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion: instead of creating a new test file, could this test go in test_struct_conversions.py test_struct_field_validation.py, whichever one you think is more appropriate?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks; the new test is intentionally isolated because it covers the newly added conversion-validation behavior and does not fit the existing fixture scope. No production issue is involved.

@github-actions

This comment has been minimized.

efegokdemir and others added 3 commits September 29, 2026 14:46
Co-authored-by: Codex <codex@openai.com>
Signed-off-by: Efe Gökdemir <efe@rexcode.co.uk>
Signed-off-by: Efe Gökdemir <efe@rexcode.co.uk>
Signed-off-by: Efe Gökdemir <efe@rexcode.co.uk>
@efegokdemir
efegokdemir force-pushed the codex/issue-11576-gpu-struct-titles branch from 715717c to fb4c8b4 Compare September 29, 2026 11:47
@efegokdemir

Copy link
Copy Markdown
Contributor Author

Rebased the existing branch onto current main and moved the NumPy field-title regression into python/cuda_cccl/tests/compute/test_struct_conversions.py, keeping the coverage with the related conversion tests.

Validation: Ruff check and format check passed; git diff --check passed. The targeted pytest could not run because this environment has neither pytest nor the CUDA test dependencies installed. The existing HostJIT segmented-reduce CI failure is unrelated to this patch and was left unchanged.

@shwina

shwina commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

/ok to test fb4c8b4

@github-actions

This comment has been minimized.

@shwina
shwina enabled auto-merge (squash) October 2, 2026 17:32
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

🥳 CI Workflow Results

🟩 Finished in 3d 05h: Pass: 100%/119 | Total: 2d 12h | Max: 2h 25m

See results here.

@shwina
shwina merged commit 50a9e53 into NVIDIA:main Oct 2, 2026
292 of 295 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[BUG]: cuda.compute: gpu_struct treats NumPy field titles as separate fields

4 participants