Skip to content

FIX: Add type check for shape elements in DeviceNDArrayBase constructor#352

Merged
gmarkall merged 5 commits intoNVIDIA:mainfrom
gnzng:fix351
Jul 25, 2025
Merged

FIX: Add type check for shape elements in DeviceNDArrayBase constructor#352
gmarkall merged 5 commits intoNVIDIA:mainfrom
gnzng:fix351

Conversation

@gnzng
Copy link
Contributor

@gnzng gnzng commented Jul 24, 2025

This PR addresses issue #351 by adding a type check to ensure all elements of the shape argument are integers during DeviceNDArrayBase construction.

Description

numba_cuda.device_array would accept a shape argument containing non-integer types, such as booleans or floats. The values were implicitly cast to floats, which could lead to unexpected behavior or downstream errors.

Behavior Before This PR

The following code would execute without raising an error, creating an array with a shape composed of floats:

import numpy as np
from numba_cuda import device_array

# No error was raised
arr = device_array(shape=np.array([True, False, 10, 10.12]))

# Resulting shape would be (1.0, 0.0, 10.0, 10.12)
print(arr.shape)

Behavior After This PR

With this change, providing non-integer elements in the shape argument now raises an immediate TypeError, ensuring that only valid shapes are used for array creation.

import numpy as np
from numba_cuda import device_array

# Now raises a TypeError
arr = device_array(shape=np.array([True, False, 10, 10.12]))

The resulting error is:

TypeError: all elements of shape must be ints

This change improves input validation and prevents the creation of improperly configured device arrays.

@copy-pr-bot
Copy link

copy-pr-bot bot commented Jul 24, 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.

@gmarkall
Copy link
Contributor

/ok to test

@copy-pr-bot
Copy link

copy-pr-bot bot commented Jul 25, 2025

/ok to test

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

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

@gmarkall
Copy link
Contributor

/ok to test 85a4211

@gmarkall
Copy link
Contributor

Thanks for the PR!

When a scalar float is passed for the shape, we get an error before the new check sees it (and I think the new check would also fail as a float is not iterable):

In [3]: cuda.device_array(1.2)

File ~/numbadev/numba-cuda/numba_cuda/numba/cuda/api.py:158, in device_array(shape, dtype, strides, order, stream)
    152 @require_context
    153 def device_array(shape, dtype=np.float64, strides=None, order="C", stream=0):
    154     """device_array(shape, dtype=np.float64, strides=None, order='C', stream=0)
    155 
    156     Allocate an empty device ndarray. Similar to :meth:`numpy.empty`.
    157     """
--> 158     shape, strides, dtype = prepare_shape_strides_dtype(
    159         shape, strides, dtype, order
    160     )
    161     return devicearray.DeviceNDArray(
    162         shape=shape, strides=strides, dtype=dtype, stream=stream
    163     )

File ~/numbadev/numba-cuda/numba_cuda/numba/cuda/api_util.py:11, in prepare_shape_strides_dtype(shape, strides, dtype, order)
      9     strides = (strides,)
     10 else:
---> 11     strides = strides or _fill_stride_by_order(shape, dtype, order)
     12 return shape, strides, dtype

File ~/numbadev/numba-cuda/numba_cuda/numba/cuda/api_util.py:16, in _fill_stride_by_order(shape, dtype, order)
     15 def _fill_stride_by_order(shape, dtype, order):
---> 16     nd = len(shape)
     17     if nd == 0:
     18         return ()


TypeError: object of type 'float' has no len()

What do you think about addressing this variant of the error too?

Would you also like to add a test of the fix(es)?

@gmarkall gmarkall self-requested a review July 25, 2025 10:36
@gmarkall gmarkall added the 4 - Waiting on author Waiting for author to respond to review label Jul 25, 2025
@gnzng
Copy link
Contributor Author

gnzng commented Jul 25, 2025

Thanks a lot!

I added the float check further upstream in prepare_shape_strides_dtype() ae2305b

Added tests for testing various inputs and input dtypes:

  • 36ed9ef (test for right errors)
  • 6274af6 (make sure list and np.arrays are still working with valid ints)

@gmarkall
Copy link
Contributor

/ok to test 6274af6

@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
@gnzng
Copy link
Contributor Author

gnzng commented Jul 25, 2025

Fix style via ruff format api_util.py since style CI ci/check_style.sh was failing a06a697

@gmarkall
Copy link
Contributor

/ok to test a06a697

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.

Many thanks!

@gmarkall gmarkall added 5 - Ready to merge Testing and reviews complete, ready to merge and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Jul 25, 2025
@gmarkall gmarkall merged commit f64c2aa into NVIDIA:main Jul 25, 2025
39 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

5 - Ready to merge Testing and reviews complete, ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants