ENH: da.ufuncs now supports DataFrame/Series by sinhrks · Pull Request #1669 · dask/dask

sinhrks · 2016-10-16T09:46:02Z

On current master, da.ufuncs doesn't work with dask.dataframe instances.

On current master:

import dask.array as da
import dask.dataframe as dd

s = pd.Series(np.random.randn(100))
ds = dd.from_pandas(s, 3)
da.sin(ds)
# ValueError: max() arg is an empty sequence

After the PR:

Now these return dask instances.

da.sin(ds)
# dd.Series<sin-cf1..., npartitions=3, divisions=(0, 34, 68, 99)>

da.sin(ds).compute()
#0    -0.329255
#1    -0.586025
#         ...   
#98    0.739737
#99    0.567551
# dtype: float64

To make impl simple, I'm adding _elemwise property to Array/_Frame which returns function used for element-wise op.

sinhrks · 2016-10-16T09:48:02Z

dask/array/ufunc.py

+    def wrapped(x, *args, **kwargs):
+        if hasattr(x, '_elemwise_'):
+            return x._elemwise_(numpy_ufunc, x, *args, **kwargs)
+            # return x._elemwise_(numpy_ufunc, x, *args, **kwargs,


Current test all succeeded even if it ignores wrap_elemwise args (like logical_and = wrap_elemwise(np.logical_and, dtype='bool')). Can I remove it?

Ah, perhaps we've gotten better at determining dtypes since when this code was originally written.

mrocklin · 2016-10-16T13:33:23Z

cc @shoyer this seems like something you might find interesting

mrocklin · 2016-10-16T13:41:10Z

This all looks pretty good to me. +1

mrocklin · 2016-10-28T21:12:57Z

This has fallen behind master. I would like to make sure that this gets in, even if it does not support all of the ufuncs.

@sinhrks if I were to rebase and merge this in is there anything that would be seriously lacking? While I would like to have frexp and modf I don't think that they should stop us from merging in.

sinhrks · 2016-10-29T12:18:55Z

@mrocklin OK. Split frexp and modf for DataFrame to #1723, which may needs little internal cleanup.

Now this PR is ready for review.

mrocklin · 2016-10-29T13:52:42Z

Two questions:

How do we get the dtype of the output series? Does some small computation happen? Are we using ._meta?
What type does da.ufunc(dd.Index) return?

sinhrks · 2016-10-29T14:55:14Z

dtype is inferred via _meta, like

np.sin(pd.Series([], dtype=np.int64))
# Series([], dtype: float64)

fixed to return dd.Index if possible. Can't work if result has bool dtype, because pd.Index returns np.ndarray rather than pd.Index (will fix in da.ufunc DataFrame support follow-up #1723)

mrocklin · 2016-10-31T13:11:46Z

Merging soon if no further comments

shoyer · 2016-10-31T16:39:34Z

dask/array/core.py

        # We don't inspect the values of 0d dask arrays, because these could
        # hold potentially very expensive calculations.
-        vals = [np.empty((1,) * a.ndim, dtype=a.dtype)
+        vals = [np.empty((1, ) * a.ndim, dtype=a.dtype)


nit: I don't think adding this extra white-space is an improvement

shoyer · 2016-10-31T16:43:22Z

dask/array/ufunc.py

+    return x.__array_wrap__(numpy_ufunc(x, *args, **kwargs))
+
+
+def wrap_elemwise(numpy_ufunc, array_wrap=False):


I don't think this will pickle properly, which will likely be an annoyance later (see pydata/xarray#901).

What the soln should be, make a callable class which wraps ufuncs like methodcaller?

We do a fair amount of this. I agree that it's not optimal but at least these are dask.array functions, rather than functions that we're trying to send over the network. Actually, I appreciate that wrapped calls Array._elemwise with the only well defined easily pickleable functions (__array_wrap__ and the provided numpy_ufunc).

I think that it's nice to avoid dynamic functions when convenient but that, in this case, it seems like the right choice. Other alternatives seem more complex. If we really wanted to ensure picklability across the project then we might consider something like toolz.curry.

Also, for what it's worth, I've started to think of cloudpickle as the default serialization solution for Python functions.

shoyer · 2016-10-31T16:47:49Z

dask/array/ufunc.py

+    def wrapped(x, *args, **kwargs):
+        if hasattr(x, '_elemwise'):
+            if array_wrap:
+                return x._elemwise(__array_wrap__, numpy_ufunc,


is __array_wrap__ defined for dask.array? Why exactly do you need this, only for some ufuncs?

This is for letting some ufuncs which not calls __array_wraps__ handles DataFrame.

np.isreal(pd.Series([1, 2, 3])) # array([ True, True, True], dtype=bool)

mrocklin · 2016-11-02T23:38:33Z

I'm going to try to release in the next two days. I may take on this PR tomorrow if you don't mind @sinhrks .

sinhrks · 2016-11-03T11:55:27Z

@mrocklin Thx for the support. Pls fix it if you already have an idea in your mind:)

mrocklin · 2016-11-03T14:15:23Z

To me it looks like all raised issues are handled. The serialization issue is long-standing and something we would need to fix throughout the codebase in many places. The __array_wrap__ question seems to have a reasonable answer. I'd like to merge this. I'll wait a few more hours to make sure that @shoyer 's time zone has woken up

sinhrks added dataframe array labels Oct 16, 2016

sinhrks commented Oct 16, 2016

View reviewed changes

sinhrks force-pushed the dask_ufunc branch 2 times, most recently from 73799c4 to dc93c1c Compare October 19, 2016 21:38

sinhrks force-pushed the dask_ufunc branch from 3d58e74 to 9abbe11 Compare October 25, 2016 21:13

sinhrks force-pushed the dask_ufunc branch 3 times, most recently from 6e1e617 to 24e320b Compare October 29, 2016 03:47

sinhrks changed the title ~~(WIP)ENH: da.ufuncs now supports DataFrame/Series~~ ENH: da.ufuncs now supports DataFrame/Series Oct 29, 2016

sinhrks mentioned this pull request Oct 29, 2016

da.ufunc DataFrame support follow-up #1723

Closed

2 tasks

sinhrks force-pushed the dask_ufunc branch from 6759b26 to 4c34b3f Compare October 30, 2016 22:51

shoyer reviewed Oct 31, 2016

View reviewed changes

sinhrks added 8 commits November 3, 2016 20:40

ENH: da.ufuncs now supports DataFrame/Series

6b9381d

Add DataFrame.__array__ interface

60c10de

Applying np.ufunc now keeps Series/DataFrame Index

d9f1e36

Call __array_wrap__ explicitly

56967f7

Add more tests

1defce6

Add more tests pt2

877419b

Fix Index

dd69c1a

rebase to master

c8f1367

sinhrks force-pushed the dask_ufunc branch from 4c34b3f to c8f1367 Compare November 3, 2016 11:54

mrocklin merged commit 225ea69 into dask:master Nov 3, 2016

sinhrks deleted the dask_ufunc branch November 3, 2016 19:32

sinhrks added this to the 0.12.0 milestone Nov 7, 2016

shoyer mentioned this pull request Nov 7, 2016

BUG: dask.array.maximum can trigger eager evaluation #1764

Closed

jakirkham mentioned this pull request Jan 19, 2022

Return da.Array rather than dd.Series for non-ufunc elementwise functions on dd.Series #8558

Merged

		return x.__array_wrap__(numpy_ufunc(x, args, *kwargs))


		def wrap_elemwise(numpy_ufunc, array_wrap=False):

Uh oh!

Conversation

sinhrks commented Oct 16, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrocklin commented Oct 16, 2016

Uh oh!

mrocklin commented Oct 16, 2016

Uh oh!

mrocklin commented Oct 28, 2016

Uh oh!

sinhrks commented Oct 29, 2016

Uh oh!

mrocklin commented Oct 29, 2016

Uh oh!

sinhrks commented Oct 29, 2016

Uh oh!

mrocklin commented Oct 31, 2016

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrocklin commented Nov 2, 2016

Uh oh!

sinhrks commented Nov 3, 2016

Uh oh!

mrocklin commented Nov 3, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sinhrks commented Oct 16, 2016 •

edited

Loading