Better error reporting in python context interface #1732
Conversation
| .. function:: context(threads, gpu_id) | ||
| :noindex: | ||
|
|
||
| Create a context that uses a set number of :attr:`threads` and the GPU with id :attr:`gpu_id`. | ||
|
|
||
| .. attribute:: threads | ||
|
|
||
| The number of threads available locally for execution, 1 by default. | ||
|
|
||
| .. attribute:: gpu_id | ||
|
|
||
| The identifier of the GPU to use, ``None`` by default. | ||
| Must be ``None``, or a non-negative integer. |
There was a problem hiding this comment.
Where did this go though?
There was a problem hiding this comment.
It was removed, and replaced with context(threads=1, gpu_id=None, mpi=None)
There was a problem hiding this comment.
There is now a single constructor, that is available regardless of which features were enabled at compile time.
If the user requests a disabled feature, i.e. GPU or MPI, an exception with a (hopefully) helpful message is thrown.
| "Construct a local context with arguments:\n" | ||
| " threads: The number of threads available locally for execution, 1 by default.\n" | ||
| " gpu_id: The identifier of the GPU to use, None by default.\n") | ||
| [](proc_allocation_shim alloc, pybind11::object mpi){ |
There was a problem hiding this comment.
I would roll the constructor from proc_allocation_shim (sans MPI) into this.
There was a problem hiding this comment.
I don't understand the request.
There was a problem hiding this comment.
There are now two context constructors that take a proc_allocation_shim, one takes an MPI object in addition. Line 100 and line 140. This could be rolled into one constructor, same as the constructor you modified.
There was a problem hiding this comment.
We have two constructors in this version:
- a constructor where the user can list the hardware (threads, gpu and mpi), with default values when they do not explicitly set one (i.e. if a gpu is not requested, the default is not to use a GPU).
- a constructor that takes a
proc_allocationand an (optional) mpi context.
Would you like to change this?
There was a problem hiding this comment.
There are 3 :) The one on line 100 is very short. But I see that I'm not reading correctly either: you can simply delete the one on line 100.
There was a problem hiding this comment.
Calling just arbor.context() will still work right?
There was a problem hiding this comment.
Actually there were 4! They have been removed.
| @@ -144,11 +144,13 @@ The Python wrapper provides an API for: | |||
|
|
|||
There was a problem hiding this comment.
At the top of this page, I would like a reminder that the availability of certain features (MPI, GPU) depend on how Arbor was built.
There was a problem hiding this comment.
This is stated at the top of the page: the first code sample shows how to use arbor.__config__ to check for features.
I have added two sentences a the very to further clarify this.
There was a problem hiding this comment.
I'd make it a note at the actual top of the page, even before the API documentation: that some options depend on compile time configuration. Repeating this here (who reads the Build and Install page after all) is not excessive.
doc/python/hardware.rst
Outdated
| @@ -8,6 +8,9 @@ Arbor provides two ways for working with hardware resources: | |||
| * *Prescribe* the hardware resources and their contexts for use in Arbor simulations. | |||
| * *Query* available hardware resources (e.g. the number of available GPUs), and initializing MPI. | |||
|
|
|||
| Note that to utilize some hardware features Arbor must be built and installed with the feature enabled, for example MPI or a GPU. | |||
| Please refer to the :ref:`kind <modelcellkind>` for informatioon on how to enable hardware support. | |||
There was a problem hiding this comment.
:ref:`kind <modelcellkind>` for informatioon -> :ref:`in_build_install` for information
Give clear error messages when a user attempts to initialise a Python context with GPU or MPI
when Arbor has not been configured with support for the respective hardware feature.
Fixes #1646.