Allow mixing dask and numpy arrays in @guvectorize by jsignell · Pull Request #6863 · dask/dask

jsignell · 2020-11-19T16:23:23Z

Tests added / passed
Passes black dask / flake8 dask
Closes Mixing of dask.array and numpy.ndarray in Numba @guvectorize #6855

I am not sure that this is the right time for adding a meta_nonempty for dask.array but I thought it might be? Also I suspect that this'll have repercussions for cupy @quasiben.

martindurant · 2020-11-26T20:36:33Z

ping @quasiben or maybe suggest someone else to review

quasiben · 2020-11-30T15:17:29Z

dask/array/utils.py

+    """
+    if not is_arraylike(x):
+        return x
+    return np.ones((1,) * x.ndim, dtype=x.dtype)


Perhaps this should use [np.ones_like[(https://numpy.org/doc/stable/reference/generated/numpy.ones_like.html) instead. cc @pentschev

If this is indeed necessary, we would probably want

dask/dask/array/utils.py

Lines 340 to 349 in 8f9fc16

def ones_like_safe(a, shape, **kwargs):

"""

Return np.ones_like(a, shape=shape, **kwargs) if the shape argument

is supported (requires NumPy >= 1.17), otherwise falls back to

using the old behavior, returning np.ones(shape, **kwargs).

"""

try:

return np.ones_like(a, shape=shape, **kwargs)

except TypeError:

return np.ones(shape, **kwargs)

here, but I'm wondering why can't we just use meta_from_array instead.

pentschev · 2020-11-30T16:16:47Z

dask/array/core.py

-        np.ones((1,) * x.ndim, dtype=x.dtype) if isinstance(x, Array) else x
-        for x in args
-    ]
+    args = [meta_nonempty(x) for x in args]


Wouldn't using meta_from_array suffice here?

My understanding is it needs to be a nonempty array for gufuncs to work. If I switch this to meta_from_array I get:

ValueError: `dtype` inference failed in `apply_gufunc`. Please specify the dtype explicitly using the `output_dtypes` kwarg. Original error is below: ------------------------ ValueError('cannot call `vectorize` on size 0 inputs unless `otypes` is set') Traceback: --------- File "/home/julia/dask/dask/array/core.py", line 377, in apply_infer_dtype o = func(*args, **kwargs) File "/home/julia/conda/envs/dask-dev/lib/python3.8/site-packages/numpy/lib/function_base.py", line 2091, in __call__ return self._vectorize_call(func=func, args=vargs) File "/home/julia/conda/envs/dask-dev/lib/python3.8/site-packages/numpy/lib/function_base.py", line 2157, in _vectorize_call res = self._vectorize_call_with_signature(func, args) File "/home/julia/conda/envs/dask-dev/lib/python3.8/site-packages/numpy/lib/function_base.py", line 2226, in _vectorize_call_with_signature raise ValueError('cannot call `vectorize` on size 0 inputs '

In that case, couldn't you just replace np.ones by ones_like_safe in the old code? That way you wouldn't even need the new meta_from_array function, unless I'm missing something.

pentschev · 2020-11-30T16:17:36Z

dask/array/utils.py

+    """
+    if not is_arraylike(x):
+        return x
+    return np.ones((1,) * x.ndim, dtype=x.dtype)


If this is indeed necessary, we would probably want

dask/dask/array/utils.py

Lines 340 to 349 in 8f9fc16

def ones_like_safe(a, shape, **kwargs):

"""

Return np.ones_like(a, shape=shape, **kwargs) if the shape argument

is supported (requires NumPy >= 1.17), otherwise falls back to

using the old behavior, returning np.ones(shape, **kwargs).

"""

try:

return np.ones_like(a, shape=shape, **kwargs)

except TypeError:

return np.ones(shape, **kwargs)

here, but I'm wondering why can't we just use meta_from_array instead.

jsignell · 2021-02-05T20:07:40Z

Thanks for the feedback @pentschev. I think I had made it overly complicated for no reason.

pentschev

Thanks @jsignell , this reads much better, and looks good to me now! 😄

jsignell · 2021-05-26T14:42:59Z

I am going to merge this if it passes.

jsignell · 2021-06-18T15:06:56Z

dask/array/utils.py

+
    try:
-        return np.ones_like(a, shape=shape, **kwargs)
+        return _array_like_safe(np.ones_like, ones_like, a, shape=shape, **kwargs)


I think this is a neccessary improvement on this function, but @pentschev please correct me if this is wrong.

No, this isn't necessary. ones_like already existed before we added like= to creation functions like np.asarray, in fact, the like= argument name comes from the *_like functions. We have added like= to all creation functions, including np.ones, but using np.ones_like is equivalent to np.ones(..., like=...). Therefore, ones_like_safe already works without the change proposed here.

Ok thank you. I will think more about what I am doing wrong then. I was getting a segfault without this change which made me think that maybe I was hitting a wierd loop or somthing. I

jsignell · 2021-06-21T15:20:55Z

Ok I made ones_like_safe operate on the _meta if available so that the result is always an evaluated array

jsignell · 2021-06-21T15:21:25Z

@jrbourbeau I plan to merge this tomorrow unless you have any comments.

pentschev

Sorry @jsignell for the late reply, overall changes look good, I suggested only using meta_from_array to get the correct _meta, hopefully that's a simpler way and will work as well.

dask/array/core.py

Co-authored-by: Peter Andreas Entschev <peter@entschev.com>

pentschev

LGTM, thanks @jsignell !

jsignell · 2021-06-29T15:50:51Z

Thanks for all the reviews! 😅

quasiben reviewed Nov 30, 2020

View reviewed changes

pentschev reviewed Nov 30, 2020

View reviewed changes

jsignell force-pushed the meta-nonempty branch from 77f1fdf to 9ae7018 Compare February 5, 2021 19:49

pentschev approved these changes Feb 5, 2021

View reviewed changes

Base automatically changed from master to main March 8, 2021 20:19

jsignell added 4 commits May 26, 2021 10:41

Make new meta_nonempty function and use it for gufunc

f0ce094

Just change Array to is_arraylike

3c15cf0

Use ones_like_safe but the shape is now the same as original

fcea296

Use ones_like_safe and make each dim size 1

582759e

jsignell force-pushed the meta-nonempty branch from 563d7e5 to 582759e Compare May 26, 2021 14:42

github-actions bot added the array label May 26, 2021

Fix rebase

6b05bff

jsignell mentioned this pull request May 26, 2021

Array: correct number of outputs in apply_gufunc #7669

Merged

3 tasks

jsignell added 2 commits June 18, 2021 10:32

Merge branch 'main' into meta-nonempty

42cf324

Make ones_like_safe safer

a7bcbfc

jsignell changed the title ~~Make new meta_nonempty function and use it for gufunc~~ Allow mixing dask and numpy arrays in @guvectorize Jun 18, 2021

jsignell commented Jun 18, 2021

View reviewed changes

Make sure that args is an evaluated array

c584f77

pentschev reviewed Jun 28, 2021

View reviewed changes

dask/array/core.py Outdated Show resolved Hide resolved

dask/array/core.py Outdated Show resolved Hide resolved

jsignell and others added 3 commits June 28, 2021 14:15

Apply suggestions from code review

7268bc6

Co-authored-by: Peter Andreas Entschev <peter@entschev.com>

Merge branch 'main' into meta-nonempty

e1e49ad

Lint

812a660

pentschev approved these changes Jun 29, 2021

View reviewed changes

jsignell merged commit 626cc72 into dask:main Jun 29, 2021

jsignell deleted the meta-nonempty branch June 29, 2021 15:50

	def ones_like_safe(a, shape, **kwargs):
	"""
	Return np.ones_like(a, shape=shape, **kwargs) if the shape argument
	is supported (requires NumPy >= 1.17), otherwise falls back to
	using the old behavior, returning np.ones(shape, **kwargs).
	"""
	try:
	return np.ones_like(a, shape=shape, **kwargs)
	except TypeError:
	return np.ones(shape, **kwargs)

Uh oh!

Conversation

jsignell commented Nov 19, 2020

Uh oh!

martindurant commented Nov 26, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pentschev Feb 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsignell commented Feb 5, 2021

Uh oh!

pentschev left a comment

Choose a reason for hiding this comment

Uh oh!

jsignell commented May 26, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsignell commented Jun 21, 2021

Uh oh!

jsignell commented Jun 21, 2021

Uh oh!

pentschev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pentschev left a comment

Choose a reason for hiding this comment

Uh oh!

jsignell commented Jun 29, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pentschev Feb 4, 2021 •

edited

Loading