Skip to content

Add "3D Auto-correlograms" (3D ACGs) postprocessing extension #3860

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

fededagos
Copy link

Following conversations at Cosyne 2025 with @alejoe91, here is a first potential implementation of the "3D autocorrelograms" as an analyzer postprocessing extension.

First introduced in Beau et al., 2025, 3D ACGs provide a rich representation of a neuron's firing rate which accounts for different firing modes.
They were also recently used by Yu et al., (2025) again as a powerful representation of firing rates that can be used e.g. by representation learning methods, the same way that Beau et al. did.

The implementation is adapted directly from NeuroPyxels (mainly maintained by @m-beau), which contains the original Python version.

The original conceptualization is from @herzfeldd, who I am also tagging here for visibility.



from spikeinterface.core.waveforms_extractor_backwards_compatibility import MockWaveformExtractor
from joblib import Parallel, delayed
Copy link
Member

Choose a reason for hiding this comment

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

@fededagos joblib is not a requirement. Instead, we normally use the built-in ProcessPoolExecutor

Is it ok if I push to this PR directly to refactor the parallelization?

Copy link
Author

Choose a reason for hiding this comment

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

Yes that is okay! Thanks

Copy link
Member

Choose a reason for hiding this comment

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

Done :) You can check the changes if you're interested!

@alejoe91 alejoe91 added the postprocessing Related to postprocessing module label Apr 11, 2025
@alejoe91 alejoe91 requested a review from zm711 April 11, 2025 15:30
Copy link
Member

@zm711 zm711 left a comment

Choose a reason for hiding this comment

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

A couple initial comments. I guess one other approach would be to try to numba this instead for a speed gain since we already use numba for the standard correlograms. I saw that you pushed changes Alessio while I was typing this so if you changed something I marked feel free to ignore.

Comment on lines 774 to 775
n_jobs : int, default: -1.
Number of parallel jobs to spawn to compute the 3D-ACGS on different units.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
n_jobs : int, default: -1.
Number of parallel jobs to spawn to compute the 3D-ACGS on different units.

If we want the job kwargs we need to do the {} trick and pull the shared kwarg docs. We can help with that.


# pre-compute time bins
winsize_bins = 2 * int(0.5 * window_ms * 1.0 / bin_ms) + 1
bin_times_ms = np.linspace(-window_ms / 2, window_ms / 2, num=winsize_bins)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
bin_times_ms = np.linspace(-window_ms / 2, window_ms / 2, num=winsize_bins)
bin_times_ms = np.linspace(-window_ms / 2, window_ms / 2, num=winsize_bins)

should we automatically make that the user can't specify a binsize that is inappropriate. So if I give a bin_size of like 0.0003 ms then most recording equipment don't have that resolution.

acgs_3d[unit_index, :, :] = acg_3d
firing_quantiles[unit_index, :] = firing_quantile
else:
# Process units in serial
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Process units in serial
# Process units serially

:)

Comment on lines 881 to 882
assert winsize_bins >= 1
assert winsize_bins % 2 == 1
Copy link
Member

Choose a reason for hiding this comment

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

We should have some error output for this case for ease of debugging in the future.

Comment on lines 10 to 16
try:
import npyx

HAVE_NPYX = True
except ModuleNotFoundError:
HAVE_NPYX = False

Copy link
Member

Choose a reason for hiding this comment

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

importlib.util.find_spec

But I haven't fixed testing yet so I can go back and fix that later to try to gain a tiny speed boost in testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
postprocessing Related to postprocessing module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants