Skip to content

[REVIEW][NFC] Vendor in serialize to allow for future CUDA-specific refactoring and changes#349

Merged
gmarkall merged 2 commits intoNVIDIA:mainfrom
atmnp:atmn/vendor-in-serialize
Jul 30, 2025
Merged

[REVIEW][NFC] Vendor in serialize to allow for future CUDA-specific refactoring and changes#349
gmarkall merged 2 commits intoNVIDIA:mainfrom
atmnp:atmn/vendor-in-serialize

Conversation

@atmnp
Copy link
Contributor

@atmnp atmnp commented Jul 23, 2025

The serialize file contain a bunch of helper functions that we may want to specialize in the future since these are used in many of our functions. This vendors them into this repo. This is an incremental NFC change (so no extra unit tests).

@copy-pr-bot
Copy link

copy-pr-bot bot commented Jul 23, 2025

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.

@atmnp
Copy link
Contributor Author

atmnp commented Jul 23, 2025

/ok to test 7669c5a

@gmarkall gmarkall added the 3 - Ready for Review Ready for review by team label Jul 24, 2025
@gmarkall
Copy link
Contributor

I think it's worth a look at Numba's test_serialize.py to see what tests there are directly testing this - perhaps we should be testing the vendored code a little more directly.

@gmarkall gmarkall added 4 - Waiting on author Waiting for author to respond to review and removed 3 - Ready for Review Ready for review by team labels Jul 24, 2025
@atmnp atmnp force-pushed the atmn/vendor-in-serialize branch from 7669c5a to fee2b53 Compare July 24, 2025 15:18
@atmnp
Copy link
Contributor Author

atmnp commented Jul 24, 2025

/ok to test fee2b53

@atmnp
Copy link
Contributor Author

atmnp commented Jul 24, 2025

Yep silly oversight, added it now. I saw the similarly named file existing in our repo and thought it'd already been checked against.

@gmarkall gmarkall added 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 4 - Waiting on author Waiting for author to respond to review labels Jul 25, 2025
Copy link
Contributor

@gmarkall gmarkall left a comment

Choose a reason for hiding this comment

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

We shouldn't bring in test cases that use the CPU target - these are all the serialize use cases. Test cases should either be modified to use the CUDA target, or not brought into Numba-CUDA, depending on how difficult it is to modify them / how much functionality they appear to cover.

@gmarkall gmarkall added 4 - Waiting on author Waiting for author to respond to review and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Jul 25, 2025
@gmarkall
Copy link
Contributor

We shouldn't bring in test cases that use the CPU target - these are all the serialize use cases. Test cases should either be modified to use the CUDA target, or not brought into Numba-CUDA, depending on how difficult it is to modify them / how much functionality they appear to cover.

I had a rethink about this - I think in the long term we should be able to get rid of serialize, since it appears to be related to pickling object mode code. Therefore, I think the existing tests are OK for now, and can be modified / removed if we make serialize-related changes.

@gmarkall
Copy link
Contributor

/ok to test 6af3ad6

Copy link
Contributor

@gmarkall gmarkall left a comment

Choose a reason for hiding this comment

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

Just waiting on CI.

@gmarkall gmarkall added 4 - Waiting on CI Waiting for a CI run to finish successfully and removed 4 - Waiting on author Waiting for author to respond to review labels Jul 30, 2025
@gmarkall gmarkall enabled auto-merge (squash) July 30, 2025 13:11
@gmarkall gmarkall merged commit d455b9b into NVIDIA:main Jul 30, 2025
41 checks passed
gmarkall added a commit to gmarkall/numba-cuda that referenced this pull request Jul 31, 2025
- [NFC] FileCheck tests check all overloads (NVIDIA#354)
- [REVIEW][NFC] Vendor in serialize to allow for future CUDA-specific refactoring and changes (NVIDIA#349)
- Vendor in usecases used in testing (NVIDIA#359)
- Add thirdparty tests of numba extensions (NVIDIA#348)
- Support running tests in parallel (NVIDIA#350)
- Add more debuginfo tests (NVIDIA#358)
- [REVIEW][NFC] Vendor in the Cache, CacheImpl used by CUDACache and CUDACacheImpl to allow for future CUDA-specific refactoring and changes (NVIDIA#334)
- [NFC] Vendor in Dispatcher as CUDADispatcher to allow for future CUDA-specific customization (NVIDIA#338)
- Vendor in BaseNativeLowering and BaseLower for CUDA-specific customizations (NVIDIA#329)
- [REVIEW] Vendor in the CompilerBase used by CUDACompiler to allow for future CUDA-specific refactoring and changes (NVIDIA#322)
- Vendor in Codegen and CodeLibrary for CUDA-specific customization (NVIDIA#327)
- Disable tests that deadlock due to NVIDIA#317 (NVIDIA#356)
- FIX: Add type check for shape elements in DeviceNDArrayBase constructor (NVIDIA#352)
- Merge pull request NVIDIA#265 from lakshayg/fp16-support
- Add performance warning
- Fix tests
- Create and register low++ bindings for float16
- Create typing/target registries for float16
- Replace Numbast generated lower_casts
- Replace Numbast generated operators
- Alias __half to numba.core.types.float16
- Generate fp16 bindings using numbast
- Remove existing fp16 logic
- [REVIEW][NFC] Vendor in the utils and cgutils to allow for future CUDA-specific refactoring and changes (NVIDIA#340)
- [RFC,TESTING] Add filecheck test infrastructure (NVIDIA#342)
- Migrate test infra to pytest (NVIDIA#347)
- Add .vscode to gitignore (NVIDIA#344)
- [NFC] Add dev dependencies to project config (NVIDIA#341)
- Allow Inspection of Link-Time Optimized PTX (NVIDIA#326)
- [NFC] Vendor in DIBuilder used by CUDADIBuilder (NVIDIA#332)
- Add guidance on setting up pre-commit (NVIDIA#339)
- [Refactor][NFC] Vendor in MinimalCallConv (NVIDIA#333)
- [Refactor][NFC] Vendor in BaseCallConv (NVIDIA#324)
- [REVIEW] Vendor in CompileResult as CUDACompileResult to allow for future CUDA-specific customizations (NVIDIA#325)
@gmarkall gmarkall mentioned this pull request Jul 31, 2025
gmarkall added a commit that referenced this pull request Jul 31, 2025
- [NFC] FileCheck tests check all overloads (#354)
- [REVIEW][NFC] Vendor in serialize to allow for future CUDA-specific
refactoring and changes (#349)
- Vendor in usecases used in testing (#359)
- Add thirdparty tests of numba extensions (#348)
- Support running tests in parallel (#350)
- Add more debuginfo tests (#358)
- [REVIEW][NFC] Vendor in the Cache, CacheImpl used by CUDACache and
CUDACacheImpl to allow for future CUDA-specific refactoring and changes
(#334)
- [NFC] Vendor in Dispatcher as CUDADispatcher to allow for future
CUDA-specific customization (#338)
- Vendor in BaseNativeLowering and BaseLower for CUDA-specific
customizations (#329)
- [REVIEW] Vendor in the CompilerBase used by CUDACompiler to allow for
future CUDA-specific refactoring and changes (#322)
- Vendor in Codegen and CodeLibrary for CUDA-specific customization
(#327)
- Disable tests that deadlock due to #317 (#356)
- FIX: Add type check for shape elements in DeviceNDArrayBase
constructor (#352)
- Merge pull request #265 from lakshayg/fp16-support
- Add performance warning
- Fix tests
- Create and register low++ bindings for float16
- Create typing/target registries for float16
- Replace Numbast generated lower_casts
- Replace Numbast generated operators
- Alias __half to numba.core.types.float16
- Generate fp16 bindings using numbast
- Remove existing fp16 logic
- [REVIEW][NFC] Vendor in the utils and cgutils to allow for future
CUDA-specific refactoring and changes (#340)
- [RFC,TESTING] Add filecheck test infrastructure (#342)
- Migrate test infra to pytest (#347)
- Add .vscode to gitignore (#344)
- [NFC] Add dev dependencies to project config (#341)
- Allow Inspection of Link-Time Optimized PTX (#326)
- [NFC] Vendor in DIBuilder used by CUDADIBuilder (#332)
- Add guidance on setting up pre-commit (#339)
- [Refactor][NFC] Vendor in MinimalCallConv (#333)
- [Refactor][NFC] Vendor in BaseCallConv (#324)
- [REVIEW] Vendor in CompileResult as CUDACompileResult to allow for
future CUDA-specific customizations (#325)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4 - Waiting on CI Waiting for a CI run to finish successfully

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants