cgen icon indicating copy to clipboard operation
cgen copied to clipboard

Enable pyright

Open alexfikl opened this issue 8 months ago • 7 comments

This enables pyright and fixed a lot of errors that it complained about (mainly around override and class variables).

alexfikl avatar Jun 07 '25 13:06 alexfikl

I can't see anything in here that would cause those loopy failures. Unrelated? Any ideas? :\

alexfikl avatar Jun 07 '25 16:06 alexfikl

Yes, totally unrelated. Loopy CI got broken by https://github.com/inducer/islpy/pull/150 and then broken a bit more by https://github.com/inducer/pymbolic/pull/186. It'll get fixed up by https://github.com/inducer/loopy/pull/937. As long as it's just the same three tests that are broken, I'm not worried.

inducer avatar Jun 07 '25 20:06 inducer

This should pass once pyopencl 2025.2 (which I've just released) is on conda-forge. Optimistically tomorrow.

inducer avatar Jun 08 '25 02:06 inducer

This should pass once pyopencl 2025.2 (which I've just released) is on conda-forge. Optimistically tomorrow.

Thanks!

alexfikl avatar Jun 08 '25 07:06 alexfikl

https://github.com/conda-forge/pyopencl-feedstock/pull/130

inducer avatar Jun 08 '25 21:06 inducer

conda-forge/pyopencl-feedstock#130

Woo :tada:

loopy still seems unhappy though, with the same 3 errors as far as I can tell? From my reading of

Differences (unified diff with -expected +actual):
    @@ -1,6 +1,9 @@
     #define lid(N) ((int) get_local_id(N))
    -...
    +#define gid(N) ((int) get_group_id(N))
    +<BLANKLINE>
    +__kernel void __attribute__ ((reqd_work_group_size(1, 1, 1))) loopy_kernel(__global float *__restrict__ a, int const n)
    +{
       for (int i_outer = 0; i_outer <= -1 + (15 + n) / 16; ++i_outer)
    -    for (int i_inner = 0; i_inner <= ((-17 + n + -16 * i_outer >= 0) ? 15 : -1 + n + -16 * i_outer); ++i_inner)
    +    for (int i_inner = 0; i_inner <= ((-16 + n + -16 * i_outer >= 0) ? 15 : -1 + n + -16 * i_outer); ++i_inner)
           a[16 * i_outer + i_inner] = (float) (0.0f);
    -...
    +}

it seems like there's a change in behavior.. that -17 became a -16. Is that ok?

EDIT: Just noticed the loopy PR isn't merged yet! Nevermind :D

alexfikl avatar Jun 09 '25 08:06 alexfikl

it seems like there's a change in behavior.. that -17 became a -16.

Yes, there is.

Is that ok?

Good question! This had me concerned, too, so I looked at these changes quite carefully. It turns out it's equivalent.

inducer avatar Jun 09 '25 13:06 inducer