Closed TomDonoghue closed 2 years ago
@ryanhammonds merging sim_modulated_signal
and sim_combined
is a really interesting idea! You're totally right they are quite similar.
I think a key element to consider for merging or not is about conceptual organization. In my mind, sim_combined
generates a signal that in a meaningful way has n
components, whereas the output of sim_modulated_signal
is arguable 1 "component" (so one could say something like, that sim_combined
signal has n
components {A1, A2,..An}, whereas I think I would say an amplitude modulated signal is basically 1 component with time-varying amplitude dynamics. In this view, despite the code similarity, it feels to me like the conceptual goal of the two is fairly different (and this is basically the same reason I'm not a huge fan of having sim_modulated_signal
in the combined
file, though I don't know where else to put it). Note that this is how I (casually) think about this - I'm not sure that this fully holds up to really pushing on the conceptual logic, or by considering the technical differences between what these functions do.
While the above gives a "shape" of the ideas here, perhaps a more practical way to poke at this is potential examples. A key element of our setup is the overall combinability of components. As a (I think) plausible example, let's take: "simulate a signal that is a combination of a powerlaw aperiodic component, and an amplitude modulated periodic component".
In it's current form, this PR allows for this:
# Label: code segment A
n_seconds = 10
fs = 1000
comps = {'sim_powerlaw' : {'exponent' : -1.5, 'f_range' : [2, None]},
'sim_modulated_signal' : {'sig_func' : 'sim_oscillation', 'sig_params' : {'freq' : 10},
'mod_func' : 'sim_oscillation', 'mod_params' : {'freq' : 1}}}
variances = [0.2, 1]
sig = sim_combined(n_seconds, fs, comps, variances)
In the above, defining all the component parameters is starting to get a bit gnarly - but it does work, and in the same way as any other combined signal.
I'm not 100% sure what the API would be for the combined combine function (lol), but I think the idea would be something like this:
# Label: code segment B
n_seconds = 10
fs = 1000
comps = {'sim_powerlaw' : {'exponent' : -1.5, 'f_range' : [2, None]},
'sim_combined' : {'components' : {'sim_oscillation' : {'freq' : 10},
'sim_oscillation' : {'freq' : 1}},
'approach' : 'multiply'}}
variances = [0.2, 1]
sig = sim_combined(n_seconds, fs, comps, variances)
As a sidenote, I wasn't sure that recursive calls would work, but the following is apparently valid / works:
# Label: code segment C
n_seconds = 10
fs = 1000
comps = {'sim_powerlaw' : {'exponent' : -1.5, 'f_range' : [2, None]},
'sim_combined' : {'components' : {'sim_powerlaw' : {'exponent' : -1},
'sim_oscillation' : {'freq' : 1}}}}
variances = [0.2, 1]
sig = sim_combined(n_seconds, fs, comps, variances)
^This suggests that if we did add the "how to combine" argument to sim_combined
this recursive aspect of it all would work.
I hadn't at all thought about directly supporting multiple modulators (and thinking about it is slightly breaking my brain). Conceptually one could serially apply modulators, but from some quick checks I don't think np.multiply
supports passing in an arbitrary number of arrays to all be multiplied together. I'm not too sure we want to even try to support direct multiple-modulation (as opposed to allowing for it as serial application), and if we do I'm not sure it would work quite so smoothly (unless I'm missing something here).
It seems to me either approach could work. Personally, (though not super strongly) my vote is leaning towards keeping the different functions, based a combination of conceptual modularity (a new / different function for a different task), and that practically, I think I prefer code segment A
from above. It feels like we could also could hit some quirks with applying multiple multiplicative combinations if we try to combine.
Thoughts?
(Apologies for the length - a lot of this was me logic'ing through this idea as I went)
I'm surprised the recursion works! Recursion can also be used for multiple modulators:
kwargs = {'sig_func':'sim_oscillation', 'sig_params':{'freq' : 10},
'mod_func':'sim_oscillation', 'mod_params':{'freq' : 1}}
sig_mod = sim_modulated_signal(n_seconds, fs, **kwargs)
sig_double_mod = sim_modulated_signal(n_seconds, fs, 'sim_modulated_signal', kwargs,
'sim_oscillation', {'freq' : 5})
And the sim_combined
equivalent, if the approach kwarg was added, could be:
components = {'sim_oscillation': {'freq': 1, 'freq': 5, 'freq':10}}
sig_double_mod = sim_combined(n_seconds, fs, components, approach='multiply')
I checked, and you're right about np.multiply
, but np.prod
would work if we wanted to support this in sim_combined
.
Maybe we could support both sim_modulated_signal
and multiplication in sim_combined
. If you think that's overkill, I'm cool with just supporting sim_modulated_signal
and merging this in as is.
To follow up on this (@ryanhammonds) - from what I understand, integrating applying modulation in sim_combined
wouldn't change anything currently in this PR, but is really a possible addition to update the API over and above what's currently added, right? I think it's a really cool code idea, but I'm not sure if it's not a little overkill.
Since I think we can split up different possible updates here, I've re-opened the issue (#292), to keep track of some related things here, including drafting a tutorial on this, and keeping open the idea of adding modulation to sim_combined
, which we can always add in a new PR. I want to get a better sense of using this approach while writing the tutorial, and I think that might help me feel out if it would be useful to extend the sim_combined
API. Does that make sense? If so, I think we can merge this PR now, and come back around to any further edits / updates.
Sounds good! Yeah, adding multiplication in sim_combined
wouldn't change anything here, so feel free to merge.
Responds to #292
Adds a function for applying amplitude modulation (
modulate_signal
) and a function to simulate amplitude modulated signals (sim_modulated_signal
).Note that the original idea was to add
sim_modulated_oscillation
, but in the end, there is nothing really specific to applying amplitude modulation to an oscillatory signal, so this function might as well be more general. The only slight quirk, is that the only real place to put the generalsim_modulated_signal
is in thecombined
file - though it's not really a combined signal...The modulate signal function applies amplitude modulation, and can be used like the following examples (note the two call signatures, allowing for a passing in a modulating signal, or a function name to create a modulating signal):
This creates signals that look like this:
The
sim_modulated_signal
function allows for directly simulating amplitude modulated signals, with the following creating the same kind of signals as above: