[WIP] Improving benchmarks #1137
Conversation
|
Can you rebase the changes to current master? it looks like the first two should not be included in this PR and clutter the files to be reviewed :/ |
Shouldn't the |
|
Ohhhh, I completely overlooked this, I though this was built on top of #1088 to merge into master, not to merge into benchmarks PR. Yes, |
I was confused whether to make this PR to |
arviz/plots/plot_utils.py
Outdated
|
|
||
| return np.arange(x_min, x_max + width + 1, width) | ||
|
|
||
|
|
||
| def _sturges_formula(dataset, mult=1): |
There was a problem hiding this comment.
I am not sure about what functions use this, but it looks like it should be moved to stats_utils like get_bins
There was a problem hiding this comment.
Currently, only rank_plot uses this
arviz/stats/stats_utils.py
Outdated
| from scipy.signal import convolve | ||
| from scipy.signal.windows import gaussian |
There was a problem hiding this comment.
are these needed? maybe they are used by fast_kde?
There was a problem hiding this comment.
Yes, there are a few more unsed imports. I'll remove them
Whatever is less work for you will be best. |
I think here you would have to always rebase this branch with master everytime you want to review so it's better if I close this and make this PR to master? I think it also resolves conflicting files :) |
Description
Fixes #782 and builds on top of the work done in #1088.
I've moved
_fast_kdeand_fast_kde_2dtokde_utilsandget_binstostats_utilsto resolve cyclic import issues occuring due to import ofhistogramfor the_fast_kde.I've also modified the benchmarks and will cover more functions soon. Currently, the
stats_variance_2dbenchmark does not work with Numba disabled but I couldn't figure out why (the function still works with Numba disabled).cc @OriolAbril, @ahartikainen
Checklist