inincs / pyNCS

pyNCS is a python library that allows easy access to Neuromorphic Chips and Systems (NCS),
http://inincs.github.com/pyNCS/
GNU General Public License v2.0
16 stars 10 forks source link

AddrGroup: spiketrain generators (and _inh_poisson in particular) #10

Closed fabioedoardoluigialberto closed 11 years ago

fabioedoardoluigialberto commented 11 years ago

Hi there,

there is a problem with generating spike trains with inhomogeneous rates.

The function spiketrains_inh_poisson returns criptic error (for the end user I mean) when base_generator is passed as argument. This is because a base_generator argument is already internally passed to spiketrains_inh_generator function. The error is a TypeError and the user can't understand the problem without having a look to the code...

If spiketrains_inh_poisson function is used the user shouldn't be allowed to specify a base_generator at all, for obvious reasons.

I propose to remove the kw_args from spiketrains_inh_poisson, because in this context it can cause problems and it's difficult to debug. If the user needs to specify a different generator or he wants to also pass more arguments to the generator then spiketrains_inh_generator should be used instead. In this case spiketrains_inh_poisson becomes just a shortcut, as I think it was supposed to be.

I see this problem related to INI specific usage of spiketrain generator (pyST and co.) so I think we should worry about what would happen to people using different interfaces and spiketrains-related packages.

What do you think?

sheiksadique commented 11 years ago

Two issues here..

Error handling: That is a global issue, if the functions are not used the way they are supposed to, either you go through the error log and figure it out or the programmer has to put in type checks at every place, the later being too much of a hassle if it has to be done for every single function.

inh_poisson vs inh_generator: I agree, if a generator is provided, it is point less to call it poisson since the distribution is dependent on the generator. If you have an implementation go ahead and commit it.

I dont think pyNCS at this point "supports" (in other words "designed to handle") any other package other than pyNCS.pyST.spikes which is a tweaked version of NeuroTools.spikes module. So lets not worry about people using different interfaces and stuff..