sim suggestions

[TomDonoghue]

I'm starting to use neurodsp.sim, and have some possible suggestions:

ToDos:

• Add in general colored noise generation (implementation available here: https://github.com/felixpatzelt/colorednoise) [Note: Richard added his own implementation].
• The 'Input Suggestion' docs for sim_oscillator or wrong / misplaced, right? Am I missing something? [Fixed: with more general doc clean ups.]
• I'll probably update some variable names, to update to our API conventions (ex - brownNf -> brown_nf or similar).

Open Questions:

• Is there any reason not to generalize all the functions that currently add 1/f^2 noise, to be able to add 1/f^n noise, with set-able n?
• Mixed conventions: some functions take in a signal length, others a number of samples. We should consolidate on one approach - any suggestions on which is better?

Anyways - I'll use this issue for continuing points of discussion, and open a PR soon-ish with some updates - so let me know of any thoughts about things here.

[rdgao]

• I have code for generating arbitrary exponent noise through a rotation, seems to be how that colorednoise repo does it. I can add it to sim.py if you want: https://github.com/rdgao/SpaceRecon/blob/master/nlds/utils.py

• I think there are currently two ways to get 1/f^2 noise in sim.py: my prefered way has been convolving with an exponential (synaptic) kernel, I think Scott's been using integrated white noise with a highpass. No real reason not to have a third way, as above, I guess there hasn't been a need to vary slope? In any case, yeah I can add that.

• yes please update variable names as per the convention. For the signal length thing, the most "friendly" user-level function, in my experience, is to take in a length (in seconds) and fs, so everything that uses real time is scaled appropriately. But under the hood, I think it almost always just converts that to a signal length in samples. I would prefer the former as far as user-facing functions go.

[srcole]

The 'Input Suggestion' docs for sim_oscillator or wrong / misplaced, right? Am I missing something?

Yeah, you're right. That was an earlier draft of inputs

Also agree that the API should be improved. My bad :)

[TomDonoghue]

@rdgao : If you have code for generating 1/f^x noise, that you can add in, that would be great! If it's the same as in the repo I linked great (if it's a different procedure, maybe we should check equivalence, explore a bit?). If you want to make a PR with that copied in, I'll start working with that updated version, and go from there with the other things I was thinking of.

[TomDonoghue]

Thanks Richard for adding the variable 1/f^beta generation.

Question: how disruptive would it be to update the API's / variable names to some degree? Is there much established code that depends on this file?

What I'm thinking, for example:

• N is used in different ways (n-samples & filter order). I suggest updating to n_samples / filt_order, as appropriate.
• Drop capitals in var names: so N_cycles -> n_cycles, for example

[srcole]

I believe those are just regular (position-based) arguments, so I don't see why changing their names would interfere with anything. I doubt anyone is using this and using those as keyword arguments.

If we do want to change a keyword argument, I think we could just add the new one as the recommended, but keep the old one as a redundant argument and throw a deprecation warning when someone uses it.

[TomDonoghue]

Okay cool - I'll go through with some changes, and we can sanity check at PR that they don't seem too drastic.

Couple other Q's @srcole :

• why does 'sim_noisy_oscillator' (& sim_noisy_bursty_oscillation) necessarily high-pass filter the background signal? Is this required for some reason? Any reason not to accept 'None' to not filter?
• ^Same functions, have no amplitude scaling of the oscillation right now? Any reason not to add that?
• ^Same functions, can I update them to accept an exponent value, and use 'sim_variable_power_law', instead of necessarily using brown noise?

[srcole]

why does 'sim_noisy_oscillator' (& sim_noisy_bursty_oscillation) necessarily high-pass filter the background signal? Is this required for some reason? Any reason not to accept 'None' to not filter?

That would be fine. I did it because I would often plot these signals, and if I'm interested in a 10Hz oscillation, I can't see it as well if a lot of the power is < 1 Hz (because there are large slow fluctuations). We generally do highpass our empirical data, right? (Though, that may be more like 0.5Hz than 2Hz).

^Same functions, have no amplitude scaling of the oscillation right now? Any reason not to add that?

The amplitude is controlled by the SNR parameter. It's a ratio of the power of the oscillation to the power of the background. I think this is appropriate because the overall voltage scale seems kind of arbitrary.

^Same functions, can I update them to accept an exponent value, and use 'sim_variable_power_law', instead of necessarily using brown noise?

Yes, that would be great :)

[TomDonoghue]

Ah, okay - amplitude thing makes total sense. I'll update for optional filtering and variable exponents!

[srcole]

While making bycycle, I fixed up the inconsistency issues with the simulation package and wrote some better smoke tests for them.

So for neurodsp v1.0.0, I think we should update sim.py here with the sim.py from bycycle v0.1.0. I'll do that in a later PR.

[TomDonoghue]

Let's coordinate: http://github.com/tomdonoghue/neurodsp/
^ I don't think I did much on testing, but WIP on a bunch of updates / generalizations for simulations, based on this issue. I'll open a PR on my stuff soon.