Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DeprecationWarning when broadcasting scalar array type over DOFArray #292

Open
nicknytko opened this issue Dec 10, 2024 · 3 comments
Open

Comments

@nicknytko
Copy link

As discussed from CS meeting earlier, I am getting a DeprecationWarning when dividing a DOFArray by an arraycontext scalar:

/Users/nnytko2/Git/ceesd-nonlinear-solve/code/actx_issue.py:17: DeprecationWarning: Broadcasting <class 'meshmode.dof_array.DOFArray'> over array context array type <class 'arraycontext.impl.pyopencl.taggable_cl_array.TaggableCLArray'> is deprecated and will no longer work in 2025. There is no replacement as of right now. See the discussion in https://github.com/inducer/arraycontext/pull/190.
  y = x / flat_norm(x) # Warning occurs here

And minimal reproducing example:

import numpy as np
import pyopencl as cl
from grudge.discretization import make_discretization_collection
from meshmode.array_context import PyOpenCLArrayContext
from meshmode.mesh.generation import generate_box_mesh
from meshmode.dof_array import flat_norm
from pytools.obj_array import flat_obj_array

ctx = cl.create_some_context()
queue = cl.CommandQueue(ctx)
actx = PyOpenCLArrayContext(queue)

mesh = generate_box_mesh((np.linspace(0, 1, 10),))
dcoll = make_discretization_collection(actx, mesh, order=1)

x = dcoll.zeros(actx) + 1.
y = x / flat_norm(x) # Warning occurs here
@inducer
Copy link
Owner

inducer commented Dec 11, 2024

Thanks for writing this up! This is an informative corner case. We started issuing a warning for this type of code because finding the array types associated with an array context (to allow broadcasting specifically for them) has a number of problems:

  • it's sometimes contradictory, in that numpy arrays can be array context arrays, and they have their own broadcasting behavior.
  • it requires us to find the array context first. This is a slow and failure-prone operation, and I would prefer to continue to avoid it.

At the same time, broadcasting device scalars is (IMO) eminently reasonable, and I feel that the above code should probably be allowed.

So this comes down to the question of whether we should create a loophole for "scalars". A dumb-and-perhaps-not-too-expensive way to detect them would be getattr(ary, "shape", None) == (). But this feels loosey-goosey in a not-so-healthy way. I'd be grateful for alternate proposals/ideas.

cc @alexfikl

@inducer
Copy link
Owner

inducer commented Dec 11, 2024

#280 barks up a similar tree, but it's still not a great solution.

@alexfikl
Copy link
Collaborator

I don't really have any new useful ideas :( At least at the time this warning was introduced, what I mostly had in mind was written here: #190 (comment).

In general, it seems like the hard part is ContainerTypeA + ContainerTypeB, where we don't know who's supposed to broadcast into who. Making ContainerTypeA + ArrayOrScalar work should be doable with some hackery (like you mentioned)..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants