Skip to content

Use math.prod instead of np.prod on lists, tuples, and iters #9232

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.

Sign up for GitHub

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

Jump to bottom
Merged
jsignell merged 3 commits into from
Jul 6, 2022
Merged

Use math.prod instead of np.prod on lists, tuples, and iters #9232

jsignell merged 3 commits into from
Jul 6, 2022

Conversation

@crusaderky
Copy link
Collaborator

@crusaderky crusaderky commented Jul 1, 2022 •
edited
Loading

math.prod requires Python 3.8+, so historically it wasn't used in the codebase.
It's 20~40x faster than numpy.prod on pure python lists and tuples, and it doesn't need to convert iterators to lists, resulting in cleaner code.

>>> a = (1,2,3)
>>> %timeit numpy.prod(a)
3.27 µs ± 51.9 ns per loop (mean ± std. dev. of 7 runs, 100,000 loops each)
>>> %timeit math.prod(a)
92.5 ns ± 0.38 ns per loop (mean ± std. dev. of 7 runs, 10,000,000 loops each)
>>> %timeit numpy.prod(list(range(1, 4)))
3.42 µs ± 19.2 ns per loop (mean ± std. dev. of 7 runs, 100,000 loops each)
>>> %timeit math.prod(range(1, 4))
159 ns ± 0.677 ns per loop (mean ± std. dev. of 7 runs, 10,000,000 loops each)

Also, numpy.prod returns a numpy.int64, which can cause problems down the line.

@github-actions github-actions bot added the array label Jul 1, 2022
@crusaderky crusaderky self-assigned this Jul 1, 2022
crusaderky
crusaderky commented Jul 1, 2022
View reviewed changes
@crusaderky crusaderky marked this pull request as ready for review July 1, 2022 12:55
jsignell
jsignell approved these changes Jul 6, 2022
View reviewed changes
Copy link
Member

@jsignell jsignell left a comment

Choose a reason for hiding this comment

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

This looks great! Really nice cleanup :)

dask/array/tests/test_array_core.py
from operator import add, sub
from threading import Lock

from numpy import nancumprod, nancumsum
Copy link
Member

@jsignell jsignell Jul 6, 2022

Choose a reason for hiding this comment

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

Thanks for doing this cleanup!

@jsignell
Copy link
Member

jsignell commented Jul 6, 2022

rerun tests

@jsignell jsignell merged commit 785c845 into dask:main Jul 6, 2022
@jsignell
Copy link
Member

jsignell commented Jul 6, 2022

Thanks for this @crusaderky!! This feels like a pure improvement.

@crusaderky crusaderky deleted the math.prod branch July 6, 2022 16:18
@Illviljan Illviljan mentioned this pull request Jul 8, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jsignell jsignell jsignell approved these changes

@pavithraes pavithraes Awaiting requested review from pavithraes

@ian-r-rose ian-r-rose Awaiting requested review from ian-r-rose

Assignees

@crusaderky crusaderky

Labels

array

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@crusaderky @jsignell