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] - Added test function for test sim_spiketrain_poisson #62

Merged

Conversation

maestapereira
Copy link
Collaborator

In the process of creating test functions for other functions that don't yet have one. Related to issue #34

What's added:

  • adds test_sim_spiketrain_poisson function for sim_spiketrain_poisson within sim\dist.py.

@maestapereira maestapereira added enhancement New feature or request 0.1 Update to go into the 0.1.0 release. labels Oct 22, 2021
@maestapereira maestapereira mentioned this pull request Oct 22, 2021
9 tasks
@TomDonoghue TomDonoghue changed the base branch from main to paper October 22, 2021 21:13
@TomDonoghue TomDonoghue changed the base branch from paper to main October 22, 2021 21:13
@TomDonoghue
Copy link
Member

Hey @maestapereira - this looks great!

I think the parameter combinations here could probably be tweaked a bit though. In particular fs=15 is low resolution (15 samples per second) especially given one of the simulations requests a firing rate of 100 Hz (100 spikes for second), which 'saturates' (there aren't enough samples to really simulate this firing rate). These parameters end up being sorta different from what the user is likely to want, and I would think we can make this a bit closer to expected use cases (making the test a somewhat better test for "normal" performance).

Specific suggestions:

  • A typical value for fs would be something like 1000. For tests, we want to keep sizes small, so I recommend a "middle ground" value (closer to realistic, but not too big) of fs=100
  • Similarly, n_samples could be a bit larger. For convenience, let's set n_samples=100. Note that between 100 fs & 100 samples, we are now simulating 1 second of data, which is convenient
  • Now, we can update the tests of the expected number of "spikes". For fr=0, it's still zero. For fr=3, the expected number of spikes is 3 (since we are now simulating one second of data). However, it's not guaranteed to be exactly three (there is some randomness). To check that the number of spikes is close to expected, we could check there is not too many (say, less than 10). Similarly, for the fr of 100, we expect close to 100 spikes (but necessarily exactly), so we could test that there is, for example, over 75 spikes. Between these tests, we roughly check that the output generally matches the given firing rate.

Does that make sense? If so, please update the parameters in the code, and then this should be good to go!

@maestapereira
Copy link
Collaborator Author

Hi @TomDonoghue!
Yes, that makes complete sense - thanks for the suggestions! I just incorporated them:)

@TomDonoghue
Copy link
Member

Awesome, looks great! Merging in now!

@TomDonoghue TomDonoghue merged commit dbf9c57 into spiketools:main Oct 22, 2021
@maestapereira maestapereira deleted the mod_test_sim_spiketrain_poisson branch October 22, 2021 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.1 Update to go into the 0.1.0 release. enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants