search

inducer / arraycontext

Choose your favorite numpy-workalike!
6 stars 11 forks source link

Defines TaggableCLArray #111

Closed kaushikcfd closed 2 years ago

kaushikcfd commented 3 years ago
inducer commented 2 years ago

Unsubscribing... @-mention or request review once it's ready for a look or needs attention.

kaushikcfd commented 2 years ago

Tests should pass once https://github.com/inducer/pyopencl/pull/531 (or something equivalent) lands.

alexfikl commented 2 years ago

Any chance this works with pytential too? That has a very "interesting" combination of using array contexts and direct calls to pyopencl, which is easy to break.

Maybe worth adding to the CI?

kaushikcfd commented 2 years ago

and direct calls to pyopencl

:nauseated_face:

Thanks, added a CI here: https://github.com/kaushikcfd/pytential/pull/1. Will see if anything "interesting" pops up.

kaushikcfd commented 2 years ago

@alexfikl: Thanks for catching that.

===== 136 failed, 65 passed, 1 skipped, 385 warnings in 828.91s (0:13:48) ======

There are quite a few failures in pytential.

Probably not a smart thing to require this type. So there are a couple of options I can do here:

  1. Fix pytential to not do this. (Unsure how involved this would be)
  2. Just keep this limited to PytatoPyOpenCLArrayContext.
alexfikl commented 2 years ago

@alexfikl: Thanks for catching that.

===== 136 failed, 65 passed, 1 skipped, 385 warnings in 828.91s (0:13:48) ======

There are quite a few failures in pytential.

Not bad! :rocket:

Probably not a smart thing to require this type. So there are a couple of options I can do here:

  1. Fix pytential to not do this. (Unsure how involved this would be)
  2. Just keep this limited to PytatoPyOpenCLArrayContext.

I imagine it would be quite a bit of work to get pytential to work. Besides calling pyopencl.array directly, it also calls loopy directly (without actx.call_loopy) and a few algorithms from pyopencl (ElementwiseKernel?).. so, yeah, a bit of work!

Would it be possible to just have the pyopencl array context handle both the tagged and untagged arrays?

kaushikcfd commented 2 years ago

I imagine it would be quite a bit of work to get pytential to work. Besides calling pyopencl.array directly, it also calls loopy directly (without actx.call_loopy) and a few algorithms from pyopencl (ElementwiseKernel?).. so, yeah, a bit of work!

Ah, thanks! Yep, I'll just go with not requiring TaggableCLArray in PyOpenCLArrayContext.

kaushikcfd commented 2 years ago

not requiring TaggableCLArray in PyOpenCLArrayContext.

Yep, doing that seems to fix the CIs in pytential as well! (Thanks!)

This PR is ready for review.

inducer commented 2 years ago

LGTM, thanks!