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] Min-to-min and max-to-max phase shifting #247

Merged
merged 4 commits into from
Feb 22, 2021
Merged

[ENH] Min-to-min and max-to-max phase shifting #247

merged 4 commits into from
Feb 22, 2021

Conversation

ryanhammonds
Copy link
Member

This expands phase shifting to allow for minima-to-minima and maxima-to-maxima sims. It also add phase shifting to sim_bursty_oscillation, which wasn't possible before. The phase kwarg was added to sim_cycle so phase shifting could be done before tiling.

The motivation for this was working with sims + bycycle and the lack of a convenient way to sim cycles from trough-to-trough or peak-to-peak. When simulating from zerox-to-zerox, but bycycle defined cycles otherwise, cyclepoints could get messy. Maybe bycycle could be updated to define cycles from zerox-to-zerox, but that will likely have other side-effects to work out.

The phase argument still accepts a float from [0, 1], but it also accepts min or max strings to automatically determine the phase shift required to sim from trough-to-trough or peak-to-peak. I'm not sure a mixed param type is the best design so I'm open to suggestions.

Simple example:

import numpy as np

from neurodsp.sim.cycles import sim_cycle
from neurodsp.plts import plot_time_series

n_seconds = 5
fs = 1000

sig_max = sim_cycle(n_seconds, fs, 'asine', phase='max', rdsym=0.75)
sig_min = sim_cycle(n_seconds, fs, 'asine', phase='min', rdsym=0.75)

times = np.arange(0, len(sig_max)/fs, 1/fs)

plot_time_series(times, [sig_min, sig_max], labels=['min', 'max'])

Bycycle cyclepoints example:

import numpy as np

from neurodsp.sim import sim_combined
from neurodsp.plts import plot_time_series

from bycycle.features import compute_cyclepoints
from bycycle.plts import plot_cyclepoints_df


np.random.seed(0)

n_seconds = 5
fs = 1000

components = {'sim_bursty_oscillation': {'freq': 10, 'cycle': 'asine',
                                         'rdsym': 0.25, 'phase': 'min'},
              'sim_powerlaw':{'exponent': -2}}

sig = sim_combined(n_seconds, fs, components)

times = np.arange(0, len(sig)/fs, 1/fs)

df_samples = compute_cyclepoints(sig, fs, (1, 50))
plot_cyclepoints_df(df_samples, sig, fs)

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

This looks like a really nice extension, thanks @ryanhammonds!

I think all the extended functionality makes sense / looks good here! I don't love the mixed input type of the phase arg, but I can't really think of any other clear / obvious way to do it, so I reckon this should be fine. I've made some comments, mostly on documentation stuff, if you want to check through and push any updates (if you agree with them).

@TomDonoghue
Copy link
Member

Thanks @ryanhammonds for the update! I direct pushed some small spelling / description tweaks.

This looks good to me - merging in!

@TomDonoghue TomDonoghue merged commit ebf531c into main Feb 22, 2021
@TomDonoghue TomDonoghue deleted the phase branch February 22, 2021 20:41
@ryanhammonds ryanhammonds mentioned this pull request May 3, 2021
26 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