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

[ENH] Optimization #201

Merged
merged 3 commits into from
Mar 28, 2021
Merged

[ENH] Optimization #201

merged 3 commits into from
Mar 28, 2021

Conversation

ryanhammonds
Copy link
Contributor

@ryanhammonds ryanhammonds commented Mar 9, 2021

Addresses a point in #193. This adds minor tweaks to speed up fitting. The most noticeable difference is when using fit_fooof_3d. 56% faster for a (100, 10, 99) array:

import numpy as np

from fooof import FOOOFGroup
from fooof.objs import fit_fooof_3d
from fooof.sim import gen_group_power_spectra

np.random.seed(0)

ap_params = [-2, 0]
pe_params = [[10, 5, 5], [25, 5, 5]]

freqs, powers = gen_group_power_spectra(10, (1, 50), ap_params, pe_params)

# Shape: (100, 10, 99)
powers_3d = np.array([powers] * 100)

fg = FOOOFGroup(verbose=False)
%%timeit
fgs = fit_fooof_3d(fg, freqs, powers_3d, n_jobs=4)

Before:

3.56 s ± 20.7 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

After:

2 s ± 19.7 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

@TomDonoghue
Copy link
Member

Thanks @ryanhammonds - this all looks great!

I checked through the code, and everything looks good - some really nice little clean ups and optimizations here.
I quick added some small code doc / notes extensions, and extended the test for fit_fooof_3d a little because when I was checking I figured it could / should do a little more checking that shapes are conserved as expected.

Merging this in now!

@TomDonoghue TomDonoghue merged commit 7c75d64 into main Mar 28, 2021
@TomDonoghue TomDonoghue deleted the optimize branch March 28, 2021 20:08
@ryanhammonds ryanhammonds mentioned this pull request Apr 6, 2021
3 tasks
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.

2 participants