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

REF: speed-up describe using numba #571

Merged
merged 8 commits into from
Apr 25, 2024
Merged

Conversation

martinfleis
Copy link
Member

Follow-up on #570

Had to reimplement mode to actually compile based on numba/numba#2959 (comment).

Timings compared to #570

only quantiles
# new 2.23 s ± 65.6 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
# old 7.96 s ± 59.5 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

no quantiles, mode
# new 1.25 s ± 7.76 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
# old 15 s ± 488 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

quantiles and mode
# new 2.48 s ± 42.9 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
# old  23 s ± 562 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

How would you set the CI envs to test numba and fallback options? I am not a big fan of having everything twice. Would 1 env without numba be enough in your experience?

Copy link

codecov bot commented Apr 24, 2024

Codecov Report

Attention: Patch coverage is 96.29630% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 97.7%. Comparing base (4037c70) to head (5b75742).
Report is 16 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #571     +/-   ##
=======================================
+ Coverage   97.4%   97.7%   +0.4%     
=======================================
  Files         26      35      +9     
  Lines       4328    5099    +771     
=======================================
+ Hits        4214    4984    +770     
- Misses       114     115      +1     
Files Coverage Δ
momepy/functional/_diversity.py 98.0% <96.3%> (ø)

@jGaboardi
Copy link
Member

How would you set the CI envs to test numba and fallback options? I am not a big fan of having everything twice. Would 1 env without numba be enough in your experience?

Yes, I think we can add a note to the README, docs, and wherever else that we strongly encourage installing & using numba. Then perhaps adding a 312-min.yml to the matrix that doesn't include numba.

ci/envs/310-oldest.yaml Outdated Show resolved Hide resolved
environment.yml Outdated Show resolved Hide resolved
@jGaboardi jGaboardi merged commit 4486126 into pysal:main Apr 25, 2024
14 checks passed
@martinfleis martinfleis deleted the describe-numba branch April 25, 2024 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants