Bug 861251 – dev-python/pyopencl: enable FEATURES=test using dev-libs/pocl to run OpenCL code

• Product: Gentoo Linux
• Status: RESOLVED FIXED

[Description Chris Kerr 2022-07-26 14:57:22 UTC]

As suggested in https://bugs.gentoo.org/861020#c1

- Remove `RESTRICT=test`
- Add `test? dev-libs/pocl` to dependencies (BDEPEND?)



Reproducible: Always

Steps to Reproduce:
n/a
Actual Results:  
n/a

Expected Results:  
n/a

[Comment 1 Chris Kerr 2022-07-26 15:04:45 UTC]

If multiple OpenCL implementations are present, we probably want to ensure that it runs the tests using pocl and doesn't try to access any GPUs. The environment variable PYOPENCL_CTX can be used for this, but its format is "{platform number}:{device number}" so we need to query the numbers.

Here is a snippet of Python that will do that:

import sys
import pyopencl
for i, platform in enumerate(pyopencl.get_platforms()):
     if platform.name == "Portable Computing Language":
         for j, device in enumerate(platform.get_devices()):
             if device.available:
                 print(f"{i}:{j}")
                 sys.exit(0)
         print("POCL has no available devices", file=sys.stderr)
         sys.exit(1)
print("PyOpenCL could not find POCL platform", file=sys.stderr)
sys.exit(1)

[Comment 2 Chris Kerr 2022-07-26 15:08:06 UTC]

The src_test function needs to run that snippet in an environment where python can import the pyopencl module from the build directory - I'm not sure how to do this. The stdout should be assigned to the PYOPENCL_CTX environment variable and this environment variable should be passed to the pytest command that actually runs the tests.

[Comment 3 Chris Kerr 2022-07-26 15:20:51 UTC]

Looking at the CI jobs in the PyOpenCL source, I found a better alternative: there is also a PYOPENCL_TEST environment variable which allows specifying devices by name. So instead of setting the PYOPENCL_CTX environment variable we should set:

PYOPENCL_TEST="portable:pthread"

[Comment 4 Chris Kerr 2022-07-26 15:55:34 UTC]

Created attachment 794636 [details, diff]
Patch to pyopencl ebuild

Partial implementation. Using this patch, the test fails because it cannot import the compiled `pyopencl._cl` module. Probably I need to add some command to ensure that python can import compiled modules from the build directory, but I am not sure how to do this.

[Comment 5 Chris Kerr 2022-07-26 17:13:40 UTC]

Created attachment 794639 [details, diff]
Patch to pyopencl ebuild

I found the solution by looking at the pyopencl CI scripts: change to the 'test' subdirectory before running pytest, so that it does not try to import the module from the current directory.

[Comment 6 Chris Kerr 2022-07-26 17:17:48 UTC]

If there is a reliable way of getting the --jobs count from $MAKEOPTS or similar, then we could also pass this in the POCL_MAX_PTHREAD_COUNT environment variable to limit the number of threads used by the pocl library for running the tests.

[Comment 7 Sam James 2022-07-27 02:40:45 UTC]

(In reply to Chris Kerr from comment #6)
> If there is a reliable way of getting the --jobs count from $MAKEOPTS or
> similar, then we could also pass this in the POCL_MAX_PTHREAD_COUNT
> environment variable to limit the number of threads used by the pocl library
> for running the tests.

multiprocessing.eclass will help.

[Comment 8 Chris Kerr 2022-07-27 09:06:51 UTC]

Created attachment 794855 [details, diff]
Patch to pyopencl ebuild

Use multiprocessing.eclass to set the pocl thread count based on MAKEOPTS.

[Comment 9 Chris Kerr 2022-07-27 09:23:14 UTC]

For future reference, when proposing updates to ebuilds like I am here, is it better to send a patch against the existing ebuild or to upload the complete .ebuild file?

[Comment 10 Marek Szuba (RETIRED) 2022-07-27 22:02:44 UTC]

Apart from the fact pushd and popd calls need a || die guard, which I'll add myself, LGTM. Thank you! Will test this shortly.

[Comment 11 Marek Szuba (RETIRED) 2022-07-27 23:03:41 UTC]

...and I've spoken too soon. I am afraid I have to ask you to add a sign-off line to this patch before we can merge it.

[Comment 12 Chris Kerr 2022-07-30 16:28:06 UTC]

Created attachment 795862 [details, diff]
Patch with signoff line

I read the copyright policy at https://www.gentoo.org/glep/glep-0076.html#certificate-of-origin and added a git Signed-Off-By line.

[Comment 13 Chris Kerr 2022-07-31 09:31:46 UTC]

Created attachment 796012 [details, diff]
Patch with merge conflicts resolved

I rebased against master and fixed a merge conflict.

[Comment 14 Chris Kerr 2022-08-18 07:39:59 UTC]

Do you need anything more from me?

[Comment 15 Larry the Git Cow 2022-08-21 01:16:50 UTC]

The bug has been closed via the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=5e3534894ed3ab98d7a6782977228b72daebdf53

commit 5e3534894ed3ab98d7a6782977228b72daebdf53
Author:     Christopher Kerr <[e-mail]>
AuthorDate: 2022-07-31 09:30:13 +0000
Commit:     Marek Szuba <[e-mail]>
CommitDate: 2022-08-21 01:16:41 +0000

    dev-python/pyopencl: Enable FEATURES=test using dev-libs/pocl as OpenCL implementation
    
    Use multiprocessing eclass to set thread count based on MAKEOPTS
    
    Closes: https://bugs.gentoo.org/861251
    Signed-off-by: Christopher Kerr <[e-mail]>
    Signed-off-by: Marek Szuba <[e-mail]>

 dev-python/pyopencl/pyopencl-2022.1.6.ebuild | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)