Skip to content

Add the @ operator to the delayed objects#3691

Merged
jcrist merged 2 commits intodask:masterfrom
hmaarrfk:feature_matmul_delayed
Jun 30, 2018
Merged

Add the @ operator to the delayed objects#3691
jcrist merged 2 commits intodask:masterfrom
hmaarrfk:feature_matmul_delayed

Conversation

@hmaarrfk
Copy link
Contributor

not sure if this passes for python 2, but here goes.

  • Tests added / passed
  • Passes flake8 dask

@hmaarrfk hmaarrfk force-pushed the feature_matmul_delayed branch 2 times, most recently from 7b885ff to 4c4f9e5 Compare June 29, 2018 22:25
@hmaarrfk hmaarrfk force-pushed the feature_matmul_delayed branch from 4c4f9e5 to e8a65c5 Compare June 29, 2018 22:34
Copy link
Member

@mrocklin mrocklin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In principle this looks good to me. Thanks @hmaarrfk !

Small note on testing and Python 2. We can wait to see how the CI system responds though first.

c = delayed(dummy())
d = delayed(dummy())

assert (c @ d).compute() == 4
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this causes a SyntaxError in Python 2 (seems likely) then we might wrap this in a string and then call eval like the following:

In [1]: x = 1 

In [2]: eval('x + x')
Out[2]: 2

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, you are right. Ok, let me try again
https://travis-ci.org/dask/dask/jobs/398449158#L1338

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW we've since changed our doctests to Python 3. ( #3744 )

@hmaarrfk hmaarrfk force-pushed the feature_matmul_delayed branch from 2408001 to dc37be0 Compare June 29, 2018 23:39
@hmaarrfk
Copy link
Contributor Author

Is there a reason against iterating through operator.__all__ and simply adding all of those functions to delayed objects?

import operator
for i in operator.__all__:
    print(getattr(operator, i))

@mrocklin
Copy link
Member

mrocklin commented Jun 30, 2018 via email

@jcrist
Copy link
Member

jcrist commented Jun 30, 2018

Is there a reason against iterating through operator.all and simply adding all of those functions to delayed objects?

We only want to support operators that can validly be delayed (e.g. __add__ but not __bool__). There's also a lot of other things in operator that we'd have to filter out (e.g. attrgetter), so an explicit list is simpler and more robust.

PR looks good, thanks! Merging.

@jcrist jcrist merged commit 84a035b into dask:master Jun 30, 2018
@hmaarrfk hmaarrfk deleted the feature_matmul_delayed branch June 30, 2018 00:26
@jakirkham
Copy link
Member

Thanks @hmaarrfk :)

@hmaarrfk
Copy link
Contributor Author

No, thank you three, I was able to quite quickly (well after a bunch of refractoring) speedup my code quite s ignificantly using dask,

I never use them, but is there a reason not to support operators like +=?

@jcrist
Copy link
Member

jcrist commented Jun 30, 2018

I never use them, but is there a reason not to support operators like +=?

These work automatically in python for things that don't mutate the object inplace. They work fine for delayed objects:

In [1]: from dask import delayed

In [2]: a = delayed(1)

In [3]: a
Out[3]: Delayed('int-d1948b47-8998-4fbf-b52b-b5989688996c')

In [4]: a += 2

In [5]: a.compute()
Out[5]: 3

@hmaarrfk
Copy link
Contributor Author

@jcrist clearly I never tried it. Python does a lot of magic sometimes ;)

convexset added a commit to convexset/dask that referenced this pull request Jul 1, 2018
….com/convexset/dask into fix-tsqr-case-chunk-with-zero-height

* 'fix-tsqr-case-chunk-with-zero-height' of https://github.com/convexset/dask:
  fixed typo in documentation and improved clarity
  Implement .blocks accessor (dask#3689)
  Fix wrong names (dask#3695)
  Adds endpoint and retstep support for linspace (dask#3675)
  Add the @ operator to the delayed objects (dask#3691)
  Align auto chunks to provided chunks, rather than shape (dask#3679)
  Adds quotes to source pip install (dask#3678)
  Prefer end-tasks with low numbers of dependencies when ordering (dask#3588)
  Reimplement argtopk to release the GIL (dask#3610)
  Note `da.pad` can be used with `map_overlap` (dask#3672)
  Allow tasks back onto ordering stack if they have one dependency (dask#3652)
  Fix extra progressbar (dask#3669)
  Break apart uneven array-of-int slicing to separate chunks (dask#3648)
  fix for `dask.array.linalg.tsqr` fails tests (intermittently) with arrays of uncertain dimensions (dask#3662)


if PY3:
Delayed._bind_operator(operator.matmul)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess we forgot about Python 3.4 (myself included). Put up a tweak in PR ( #3791 ), which should allow this to work on Python 3.4 as well.

@hmaarrfk
Copy link
Contributor Author

hmaarrfk commented Jul 20, 2018 via email

@jakirkham
Copy link
Member

Good point. Thanks. Missed that.

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

Successfully merging this pull request may close these issues.

4 participants