Skip to content

Conversation

@ashutoshvarma
Copy link
Contributor

Closes #6697

I was not sure about where tests for this should go, for now, the test is in numba/cuda/tests/cudadrv/test_array_attr.py.

@gmarkall gmarkall added 3 - Ready for Review CUDA CUDA related issue/PR labels Feb 8, 2021
@gmarkall gmarkall added this to the Numba 0.54 RC milestone Feb 8, 2021
Copy link
Member

@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 for tracking down this issue and making the PR! I think it is looking great in general:

  • Tests pass for me locally with hardware and the simulator.
  • The reproducer given in #6697 behaves correctly with this PR (it prints out int16 three times instead of int16 twice then float64).
  • The starter patch from #6563 applied on top of this PR passes all tests, and also allows the example from #6563 (comment) to run correctly.

So, this PR really moves things in the right direction for both the issue #6697 and the feature request #6563.

Regarding the location of the test - the majority of tests that test device arrays, transferring them between the host and the device, and checking their dtypes and other properties are in numba/cuda/tests/cudadrv/test_cuda_ndarray.py - would you be happy to move the test there?

Many thanks again!

move from test_array_attr.py to test_cuda_ndarray.py
@ashutoshvarma
Copy link
Contributor Author

Thanks for the review @gmarkall
I moved the test to numba/cuda/tests/cudadrv/test_cuda_ndarray.py (TestCudaNDArray) as you requested.

@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 Feb 9, 2021
Copy link
Member

@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 for the update - this is looking good to me!

@esc Could this have a CUDA buildfarm run please?

@esc
Copy link
Member

esc commented Feb 9, 2021

@gmarkall sure numba_smoketest_cuda_yaml_10

@esc
Copy link
Member

esc commented Feb 9, 2021

Screen Shot 2021-02-09 at 18 19 06

BF passed.

@esc esc added the BuildFarm Passed For PRs that have been through the buildfarm and passed label Feb 9, 2021
@gmarkall gmarkall removed the 4 - Waiting on reviewer Waiting for reviewer to respond to author label Feb 9, 2021
@gmarkall
Copy link
Member

gmarkall commented Feb 9, 2021

@stuartarchibald @sklam Do you want to look at this before it goes to RTM?

Copy link
Contributor

@stuartarchibald stuartarchibald left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the patch. Thanks for reviewing @gmarkall .

@stuartarchibald stuartarchibald added the 5 - Ready to merge Review and testing done, is ready to merge label Feb 10, 2021
@esc esc merged commit 720f947 into numba:master Feb 10, 2021
@gmarkall
Copy link
Member

Many thanks for the contribution @ashutoshvarma !

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

Labels

5 - Ready to merge Review and testing done, is ready to merge BuildFarm Passed For PRs that have been through the buildfarm and passed CUDA CUDA related issue/PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wrong dtype when using np.asarray on DeviceNDArray

4 participants