MSTDP(ET) implementation

[Huizerd]

Implementation of MSTDP/MSTDPET and LIF nodes from the paper by R. Florian (2007). Should I include one of the experiments as example as well? Resolves #217.

[djsaunde]

You can include the experiment if you'd like, but it may need maintenance as BindsNET changes. Alternatively, you can add it to the experiments repo.

[djsaunde]

Looks good, excellent work! Just a few minor changes.

[dee0512]

Isn't the florian node same as the LIF nodes that we have?

[Hananel-Hazan]

We currently don't have support for different dt . It will create a problematic situation where one part of the network excute one iteration as 1ms and other part will execute the same iteration on different dt. It will create unsync steps between parts of the network

…

On Thu, Mar 28, 2019, 17:01 Daniel Saunders ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In bindsnet/learning/__init__.py <#219 (comment)> : > @@ -119,10 +119,12 @@ def __init__(self, connection: AbstractConnection, nu: Optional[Union[float, Seq 'This learning rule is not supported for this Connection type.' ) - def _connection_update(self, **kwargs) -> None: + def _connection_update(self, dt: float, **kwargs) -> None: Yeah, that would work. You could do that for Nodes and LearningRule objects, too. But then you'd want a function Network.set_dt() which propagated changes to child objects. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#219 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADxYGwuIsLLi5YEFp9Cqmgjw6382MNjcks5vbS2KgaJpZM4cPbxF> .

[djsaunde]

We currently don't have support for different dt . It will create a problematic situation where one part of the network excute one iteration as 1ms and other part will execute the same iteration on different dt. It will create unsync steps between parts of the network

We are not proposing multiple dts.

[Huizerd]

Two more questions I have:

• I tried to convert the _conv2d_connection_update variants as well, but didn't really understand the way the kernels etc. are implemented. Do you want me to change these too, and if so, could you help out?
• I was wondering what the units of the time constants used everywhere are. I took them to be milliseconds, cause that's the way the papers do it, but I also saw this comment: https://github.com/Hananel-Hazan/bindsnet/blob/646dd7de73a1658bb622755e8bb3ebc406a265cd/bindsnet/network/__init__.py#L91

[djsaunde]

I tried to convert the _conv2d_connection_update variants as well, but didn't really understand the way the kernels etc. are implemented. Do you want me to change these too, and if so, could you help out?

Yeah, I can take a look at the _conv2d_connection_update variants once this is merged in. Unless you want to give it a try.

I was wondering what the units of the time constants used everywhere are. I took them to be milliseconds, cause that's the way the papers do it...

Really, the dt is dimensionless, but we usually refer to them as milliseconds and have designed the rest of the package around it. E.g., decay rates of voltages, spike traces, etc. are all at "biologically plausible" scales in milliseconds. So, we usually communicate about dt in terms of milliseconds, but we take the real-time implications with a grain of salt. That is, the simulations probably need lots of tuning before they well-approximate real neurons / synapses / learning rules.

Regarding that doc-string, that should probably be changed.

[Huizerd]

Ok, thanks. Would be great if you could look at the convolutions. I'll change the doc string right away.

[Huizerd]

@dee0512 You're right, with a different decay the FlorianNodes are pretty much similar to the LIFNodes. Should I remove them?

Another question: in the LIFNodes, the argument decay is not used as a time constant (as is written in the arguments description).
https://github.com/Hananel-Hazan/bindsnet/blob/646dd7de73a1658bb622755e8bb3ebc406a265cd/bindsnet/network/nodes.py#L328
Should this be changed?

[djsaunde]

Another question: in the LIFNodes, the argument decay is not used as a time constant (as is written in the arguments description).

@Huizerd Yeah,I've been meaning to fix this for a while. We've been using a kind of approximation (dt * self.decay is approx. torch.exp(-dt * self.decay)). We should also invert the units of time for the decay param (e.g., 20 instead of 0.05, 100 instead of 0.01, etc.), which would change torch.exp(-dt * self.decay) to torch.exp(-dt / self.decay). Let's fix this in another PR, this one's getting quite large.

...with a different decay the FlorianNodes are pretty much similar to the LIFNodes. Should I remove them?

If you can use the LIFNodes and remove the FlorianNodes with the MSTDP(ET) demo still working, that would be great.

[djsaunde]

@Huizerd I added MSTDP(ET) implementations for Conv2dConnections / did code cleanup / added some tests. Let me know if you have any thoughts.

[djsaunde]

Once the FlorianNodes are removed, let's get this merged.

[Huizerd]

Ok, all set! I will open a new PR for the time constants issue, and I also worked on passing on rewards.

[djsaunde]

Merged! Thanks for the great work.