Skip to content

Conversation

@isVoid
Copy link
Collaborator

@isVoid isVoid commented Dec 4, 2025

In separate registry mode, new instances of typing and target registries are created and added to CUDA's third party registry separately. This is in contrast to directly registerying to cudaimpl registry.

We expose most of the register functions, except lower_cast, which should also be shadowed by the custom target registry.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 4, 2025

Walkthrough

The pull request refactors how lower_cast is accessed within the static registry setup. Instead of importing lower_cast directly from numba modules, it is now obtained as an attribute from the target registry, consolidating the dependency handling and simplifying the import surface.

Changes

Cohort / File(s) Summary
Registry lower_cast refactoring
numbast/src/numbast/static/renderer.py
Added binding to expose lower_cast from target registry in SeparateRegistrySetup; removed direct import of lower_cast from numba.core.imputils
Lower_cast dependency cleanup
numbast/src/numbast/static/struct.py
Removed import of lower_cast from _render_lowering method in StaticStructConversionOperatorRenderer

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Straightforward dependency refactoring applied consistently across two files
  • Change involves removing direct imports and introducing a single attribute binding
  • Verify that target_registry.lower_cast properly exposes the functionality previously imported directly

Poem

🐰 A registry binds what once roamed free,
lower_cast finds its rightful place with glee,
Imports retire, dependencies align,
Cleaner code flows in design so fine! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: switching to use TargetRegistry.lower_cast in separate registry mode instead of importing lower_cast directly.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 22f06ec and cd51469.

📒 Files selected for processing (2)
  • numbast/src/numbast/static/renderer.py (2 hunks)
  • numbast/src/numbast/static/struct.py (0 hunks)
💤 Files with no reviewable changes (1)
  • numbast/src/numbast/static/struct.py
🔇 Additional comments (2)
numbast/src/numbast/static/renderer.py (2)

345-345: Verify that lower_cast is accessible through the registry attribute on line 25.

The import simplification removes lower_cast from the direct import on line 345. Confirm that:

  • Line 25 actually defines the new access pattern for lower_cast as a registry attribute
  • All usages of lower_cast in this file (separate registry mode) use the new pattern
  • Related changes in struct.py and other files are consistent with accessing lower_cast through the registry

Without access to verify the implementation details, the approval cannot be confirmed.


25-25: Verify that Registry has a lower_cast attribute.

The addition of lower_cast as a registry attribute is consistent with how other lowering functions (lower, lower_getattr, lower_constant) are accessed. This consolidates the API surface nicely.

However, please verify that numba.core.imputils.Registry actually provides a lower_cast attribute in the version of Numba being used.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@isVoid isVoid merged commit d52dcff into NVIDIA:main Dec 4, 2025
26 checks passed
@isVoid isVoid mentioned this pull request Dec 11, 2025
isVoid added a commit that referenced this pull request Dec 11, 2025
Numbast 0.6.0 updates numba-cuda pinnings to 0.21.0+

- bump 0.6.0
- Eagerly use all vended `numba.cuda` modules, Bump supported Numba-CUDA
version to 0.21+ (#239)
- Support Functions Typed with `__nv_bfloat16_raw` (#262)
- Use `TargetRegistry.lower_cast` in separate registry mode (#263)
- 📝 Add docstrings to `fea-enum-prefix-removal` (#261)
- Allow Prefix Removal for Struct and Enums (#259)
- Enable CodeRabbit Auto Review (#257)
- use readme.md as PYPI description readme (#254)

Co-authored-by: Michael Wang <isVoid@users.noreply.github.com>
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.

1 participant