search

inducer / pyopencl

OpenCL integration for Python, plus shiny features
http://mathema.tician.de/software/pyopencl
Other
1.07k stars 242 forks source link

Manage bit-fields #214

Open kif opened 6 years ago

kif commented 6 years ago

The PR #212 fixed a bug in pyopencl.device_type.to_string(value) as device_type is defined as a bit-field. There are 7 types stored as bit-field:

Shouldn't they return a list of string with the content ? What method name ? "interpreted_bitfield" ?

inducer commented 6 years ago

Why not update to_string?

kif commented 6 years ago

On Mon, 18 Dec 2017 16:08:48 +0000 (UTC) Andreas Klöckner notifications@github.com wrote:

Why not update to_string?

This would change the behaviour of the function which, instead of returning one string would return a list of strings and break the API.

It would probably better to create another function for the bitfields constants and deprecate (or add a warning) to the existing method.

But it is up to you.

inducer commented 6 years ago

Oh OK. Yeah, I agree we should not change the output data type. But we could change the function to return a more complicated string, e.g. GPU|ACCELERATOR|xyz. I wouldn't be opposed to adding a bits_to_string_list in support of that.

On an unrelated note, I've created an account for you with access to https://gitlab.tiker.net/inducer/pyopencl. (check your email) Would you mind submitting future PRs there to ensure that the CIs get a chance to run? Thanks!

kif commented 6 years ago

On Mon, 18 Dec 2017 19:49:57 +0000 (UTC) Andreas Klöckner notifications@github.com wrote:

Oh OK. Yeah, I agree we should not change the output data type. But we could change the function to return a more complicated string, e.g. GPU|ACCELERATOR|xyz. I wouldn't be opposed to adding a bits_to_string_list in support of that.

Seams reasonable. I will propose something like this.

On an unrelated note, I've created an account for you with access to https://gitlab.tiker.net/inducer/pyopencl. (check your email) Would you mind submitting future PRs there to ensure that the CIs get a chance to run? No problem.