Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow importing numba-dpex even if no SYCL device is detected by dpctl #1272

Merged
merged 3 commits into from
Jan 9, 2024

Conversation

diptorupd
Copy link
Contributor

@diptorupd diptorupd commented Jan 9, 2024

  • Have you provided a meaningful PR description?

Numba-dpex checks at module importation that at least one non-host SYCL device is available on the system. The check leads to the following ImportError if we try to import numba-dpex on a system that has no such device:

Traceback (most recent call last):
  File "/localdisk/work/diptorup/devel/numba-dpex/numba_dpex/config.py", line 40, in <module>
    HAS_NON_HOST_DEVICE = dpctl.select_default_device()
  File "dpctl/_sycl_device_factory.pyx", line 320, in dpctl._sycl_device_factory.select_default_device
  File "dpctl/_sycl_device_factory.pyx", line 335, in dpctl._sycl_device_factory.select_default_device
dpctl._sycl_device.SyclDeviceCreationError: Default device is unavailable.
Traceback (most recent call last):
  File "/localdisk/work/diptorup/devel/numba-dpex/numba_dpex/examples/kernel/vector_sum.py", line 8, in <module>
    import numba_dpex as ndpx
  File "/localdisk/work/diptorup/devel/numba-dpex/numba_dpex/__init__.py", line 114, in <module>
    if config.HAS_NON_HOST_DEVICE:
  File "/localdisk/work/diptorup/devel/numba-dpex/numba_dpex/config.py", line 33, in __getattr__
    return getattr(config, name)
AttributeError: module 'numba.core.config' has no attribute 'HAS_NON_HOST_DEVICE'

The check is a legacy of the time when numba-dpex needed to access a default queue to determine where a kernel should be launched if the arguments to the kernel where NumPy arrays. Since numba-dpex does not any longer support launching kernels with NumPy ndarray arguments and all offload target decisions are based on the compute follows data programming model, the check can be removed.

The PR removes the check. Without the check a user will get the exception at the array creation stage from the array library (dpnp or dpctl.tensor). The behavior is more intuitive to an user and correctly points out the reason for the exception.

Traceback (most recent call last):
  File "/localdisk/work/diptorup/devel/numba-dpex/numba_dpex/examples/kernel/vector_sum.py", line 45, in <module>
    main()
  File "/localdisk/work/diptorup/devel/numba-dpex/numba_dpex/examples/kernel/vector_sum.py", line 34, in main
    a = dpnp.random.random(N)
  File "/nfs/site/home/diptorup/.conda/envs/dpex-devel/lib/python3.10/site-packages/dpnp-0.12.0-py3.10-linux-x86_64.egg/dpnp/random/dpnp_iface_random.py", line 1214, in random
    return random_sample(size=size, device=device, usm_type=usm_type, sycl_queue=sycl_queue)
  File "/nfs/site/home/diptorup/.conda/envs/dpex-devel/lib/python3.10/site-packages/dpnp-0.12.0-py3.10-linux-x86_64.egg/dpnp/random/dpnp_iface_random.py", line 1316, in random_sample
    rs = _get_random_state(device=device, sycl_queue=sycl_queue)
  File "/nfs/site/home/diptorup/.conda/envs/dpex-devel/lib/python3.10/site-packages/dpnp-0.12.0-py3.10-linux-x86_64.egg/dpnp/random/dpnp_iface_random.py", line 106, in _get_random_state
    sycl_queue = dpnp.get_normalized_queue_device(device=device, sycl_queue=sycl_queue)
  File "/nfs/site/home/diptorup/.conda/envs/dpex-devel/lib/python3.10/site-packages/dpnp-0.12.0-py3.10-linux-x86_64.egg/dpnp/dpnp_iface.py", line 375, in get_normalized_queue_device
    return dpt._device.normalize_queue_device(sycl_queue=sycl_queue, device=device)
  File "/nfs/site/home/diptorup/.conda/envs/dpex-devel/lib/python3.10/site-packages/dpctl/tensor/_device.py", line 171, in normalize_queue_device
    d = Device.create_device(d)
  File "/nfs/site/home/diptorup/.conda/envs/dpex-devel/lib/python3.10/site-packages/dpctl/tensor/_device.py", line 76, in create_device
    _dev = dpctl.SyclDevice()
  File "dpctl/_sycl_device.pyx", line 348, in dpctl._sycl_device.SyclDevice.__cinit__
dpctl._sycl_device.SyclDeviceCreationError: Could not create a SyclDevice from default selector
  • Have you added a test, reproducer or referred to an issue with a reproducer?

The test for this change is not easily testable using pytest. Instead a new smoke test was added to the Github conda-package workflow.

  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?
  • If this PR is a work in progress, are you filing the PR as a draft?

Fixes #304

@diptorupd diptorupd requested a review from chudur-budur January 9, 2024 17:59
Copy link
Contributor

@chudur-budur chudur-budur left a comment

Choose a reason for hiding this comment

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

LGTM!

sub_group_barrier,
)

DEFAULT_LOCAL_SIZE = []
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we still have this? We need to completely nuke the old way of setting the kernel lauch parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. DEFAULT_LOCAL_SIZE is referenced in the vectorizers.py module. The vectorizers.py is badly broken and non functional at the moment that is why I did not remove the code from __init__.py. I am going to do that in a follow up PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created #1273 to keep track of the pending clean up.

@diptorupd diptorupd merged commit d12331e into main Jan 9, 2024
40 of 44 checks passed
@diptorupd diptorupd deleted the fix/import-dpex-even-without-sycl-devices branch January 9, 2024 19:19
github-actions bot added a commit that referenced this pull request Jan 9, 2024
…t-sycl-devices

Allow importing numba-dpex even if no SYCL device is detected by dpctl d12331e
github-actions bot added a commit to chudur-budur/numba-dpex that referenced this pull request Jan 9, 2024
…even-without-sycl-devices

Allow importing numba-dpex even if no SYCL device is detected by dpctl d12331e
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.

Allow importing numba-dpex even if dpctl shows that there are no SYCL platforms
2 participants