Closed sanjayankur31 closed 9 years ago
Hi Ankur, this looks great! Also the set_mode could be useful. I guess I wouldn't go as far as a constructor, but a set_mode method might be helpful for some. I have two more small remarks. Currently your AdEx model does not implement refractoriness. However, it still keeps references to "ref" the ushort vector that TIFGroup uses to implement absolute refractoriness. You might want to consider changing that. For most other neuron models (IFGroup) I am using a shared absolute/relative refractoriness mechanisms (threshold moves above reversal potential) - this again depends a bit on what you want to do. One more remark. The variable delta_t should be labeled deltaT instead of followed by _t which is reserved for type names. Could you quickly change that then I will merge your pull request.
I also created a develop branch in which we should merge this type of new developments in the future.
Cheers!
Friedemann
Ah, yeah, the adex equations themselves handle refractoriness via adaptation and threshold modification - the original model that I'm looking at doesn't include a manual system to handle it. I'll remove that part, since it's uneeded?
I'll update the variable name too.
This implementation is completely in biophysical SI units. I think I should work a bit more on it and change everything to natural units, for both consistency and performance sake before it's merged into auryn?
Ah, yeah, the adex equations themselves handle refractoriness via adaptation and threshold modification - the original model that I'm looking at doesn't include a manual system to handle it. I'll remove that part, since it's uneeded?
Yes. You can remove any reference to the ushort vector "ref".
This implementation is completely in biophysical SI units. I think I should work a bit more on it and change everything to natural units, for both consistency and performance sake before it's merged into auryn?
I am not against having SI units in the NeuronGroup code. That's actually good. However, the synaptic weight will have to be understood in the natural units of the membrane ... I hope that makes sense. Maybe an entry in the wiki on this topic is in order ... when I get around to it ;-)
Hey Ankur, just had some time and merged your pull request to the develop branch. I removed the references to "ref" and renamed the group to AdEx because that's how I guess the people know it. I hope you are fine with that. At some point we should think about vectorizing the group. Currently it's written linearly. I guess it would benefit by roughly a factor of 4x by vectorization. Would need to find an efficient sse instruction for the exp... Best, Friedemann
That's perfect. I actually modified the code to use natural units. I'll open another pull request for that asap. I'm afraid I don't know much about the sse instruction that you're speaking about. Would you have a link I could study up?
I put a TODO with a link to some fast exp implementations. The disadvantage is that we would have to link against yet another library. For examples on auryn_vector functions written using SSE intrinsics checkout out auryn_definitions.cpp Cheers!
Ran tests with different parameters and the model appears to work as expected. The implmentation can probably be optimised to reduce the number of instructions used in the evolve method.
Another thing I was thinking about was a "set_mode" method that will let users set the neurons to different modes, like regular spiking, or bursting and so on? A constructor with this option would also be nice?