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

[MRG] Simulate spikes / action potentials #259

Merged
merged 6 commits into from
Jun 3, 2021
Merged

[MRG] Simulate spikes / action potentials #259

merged 6 commits into from
Jun 3, 2021

Conversation

ryanhammonds
Copy link
Member

@ryanhammonds ryanhammonds commented Apr 20, 2021

This adds action potential cycles as the sum of two inverted asymmetrical gaussians:

import matplotlib.pyplot as plt
import numpy as np
from neurodsp.sim.cycles import sim_ap_cycle

n_seconds = 1
fs = 1000

cycle_params = {'centers': (.25, .5), 'stds':(.15, .12),
                'alphas':(4, .2), 'heights': (15, -5)}

cyc = sim_ap_cycle(n_seconds, fs, **cycle_params)

plt.plot(cyc)

ap_cycle

@ryanhammonds
Copy link
Member Author

ryanhammonds commented Apr 20, 2021

This func can be used with scipys curve_fit to fit isolated APs from real data (bycycle-tools/bycycle#100). The resulting 8 params could be useful (i.e. dimensionality reduction for APs) and included in df_features returned from bycycle.Spikes.fit().
ap_cycle_fit

neurodsp/sim/cycles.py Show resolved Hide resolved
neurodsp/sim/cycles.py Outdated Show resolved Hide resolved
@TomDonoghue
Copy link
Member

This looks super cool!

I had a quick scan of the code, left a coupe comments, and here's a couple quick "higher-level" questions:

  • It's not clear to me that the AP sim is a cycle, per se. Would it fit better in sim.transients?
  • I presume changing the gaussian code to not use scipy is about getting more control over height, center, etc? Setting height feels like it overlaps with normalizing the signal, and center feels a little weird to have in the cycles, because it makes looks like it 'cuts off' the edge of a gaussian? Am I right in thinking that one would probably never want to set center to a custom value because it would create weird concatenation artifacts?

Between the two comments, I think it's possible it might be a cleaner split to put some stuff in transients, that might work well for generating APs, but might not be quite the same as what one wants for a cycle? What do you think?

@ryanhammonds
Copy link
Member Author

ryanhammonds commented Apr 22, 2021

Thanks for the review!

  • I think it makes sense to refactor into transients with an alias in cycles so it can still be used with sim_bursty_oscillation.
  • The main reason for the sim_gaussian_cycle changes was to have control over the cycle center, which is important only for skewed cycles. Here's an example of how center makes a difference with skewed cycles:

skew_center

The height param is a bit different from variance. I'm not sure there is a clear mapping between these params unless we were to compute the mean value required to get a zero baseline.
variance_v_height

@ryanhammonds ryanhammonds changed the title [ENH] Simulate spikes / action potentials [WIP] Simulate spikes / action potentials Apr 27, 2021
@ryanhammonds
Copy link
Member Author

ryanhammonds commented Apr 27, 2021

I've update the code here based on the review. I also update the ap func to take iterables for simulation parameters. This is because 3 gaussians may desired in some cases (i.e. the sum of conductive, sodium and potassium currents). Despite this, 3 currents can be created with only 2 gaussians (where the positive portion has a wide std).

The 3 current model is from this paper. Third column plot:
img_gold

from neurodsp.sim.cycles import sim_ap_cycle
import matplotlib.pyplot as plt

cycle = sim_ap_cycle(1, 100, (.5, .3, .6), (.1, .1, .1), (0, -1, 1), (-5, 1, 2.5))

plt.plot(cycle)

example_double_skew

@ryanhammonds ryanhammonds mentioned this pull request May 3, 2021
26 tasks
@ryanhammonds
Copy link
Member Author

@TomDonoghue I made a final round of updates. There have been several changes since the last time you reviewed, but it should be ready to merge. Let me know if you'd like to look over the changes, otherwise I'll merge.

@ryanhammonds ryanhammonds changed the title [WIP] Simulate spikes / action potentials [MRG] Simulate spikes / action potentials May 27, 2021
@TomDonoghue
Copy link
Member

TomDonoghue commented Jun 2, 2021

Edit: hey @ryanhammonds, this looks great! Very cool addition!

Everything looks good to me - you can go ahead and merge when you're ready!

@ryanhammonds ryanhammonds merged commit a385e73 into main Jun 3, 2021
@TomDonoghue TomDonoghue deleted the ap branch June 3, 2021 20:06
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