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

use all available cores in feature extraction #176

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

anilbey
Copy link
Contributor

@anilbey anilbey commented Apr 4, 2024

With this change the Cell object calls the recording.compute_efeatures in parallel in the extract_efeatures function.

@codecov-commenter
Copy link

Codecov Report

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

Project coverage is 73.29%. Comparing base (b5fc0e5) to head (0b3eb99).

Files Patch % Lines
bluepyefe/cell.py 90.90% 2 Missing ⚠️
bluepyefe/rheobase.py 77.77% 2 Missing ⚠️
tests/test_efel_settings.py 66.66% 2 Missing ⚠️
bluepyefe/extract.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #176      +/-   ##
==========================================
- Coverage   73.76%   73.29%   -0.48%     
==========================================
  Files          38       39       +1     
  Lines        2413     2430      +17     
==========================================
+ Hits         1780     1781       +1     
- Misses        633      649      +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -54,20 +54,7 @@ def test_efel_strictstim(self):
}
)

self.assertEqual(self.cell.recordings[0].efeatures["Spikecount"], 0.)

def test_efel_threshold(self):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

duplicate of another test with the same name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the other test has more checks

@anilbey anilbey marked this pull request as ready for review April 11, 2024 09:59
self.assertEqual(recording.efeatures["Spikecount"], 0.)
self.assertLess(abs(recording.efeatures["AP1_amp"] - 66.68), 0.01)
Copy link
Contributor Author

@anilbey anilbey Apr 11, 2024

Choose a reason for hiding this comment

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

this code was not getting executed for a long time. The other test was shadowing it

@anilbey anilbey changed the title attempt to feature extraction in parallel use all available cores in feature extraction Apr 11, 2024
@darshanmandge
Copy link
Contributor

Thanks @anilbey !
I think we should test this with all the readers: ibw, different types of nwb, abf, etc. with a good number of ephys files to see if it works and if it improves the performance. e.g. for ibw files this bluepyemodel example would be a sufficient test.

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.

3 participants