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

Fix to input_type node of CubaLIF neurons #107

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

DylanMuir
Copy link
Collaborator

CubaLIF neurons would have an input_type node of [], instead of [1], when created from a node with scalar v_threshold. This pull request fixes this issue.

Also adds a squeeze() to the rockpool RNN notebook.

@stevenabreu7
Copy link
Collaborator

stevenabreu7 commented Jun 10, 2024

Looks good to me, thanks for the PR @DylanMuir!

Edit: actually, the v_threshold should not be a scalar, but an array of the same size as the neuron population. Can you explain where you get the NIR graph that contains a CubaLIF neuron with scalar v_threshold? We should fix it there. The current fix could also be okay but it assumes that CubaLIF neurons are 1D populations, but we may want to have multi-dim populations e.g. in SCNNs.

@DylanMuir
Copy link
Collaborator Author

Hi @stevenabreu7 , it occurred precisely when the neuron population contains 1 neuron. In other cases v_threshold was a vector as expected. I did want to check with you about 1D or 2D populations.

The alternative fix would be to ensure v_threshold is a vector with a single element, when creating a single neuron.

@DylanMuir
Copy link
Collaborator Author

Explicitly, it occurred when exporting a net to nir from snntorch, containing Synaptic modules.

in export_nir.py line 58, in _extract_snntorch_module():

vthr = module.threshold.detach().numpy()

The line looks fine, but becomes a scalar numpy array, instead of a single-element vector. So I guess it's up to you whether to fix this in NIR or ask Jason for a fix ;)

@DylanMuir
Copy link
Collaborator Author

@stevenabreu7 @Jegp Any thoughts on how to proceed here?

@Jegp
Copy link
Collaborator

Jegp commented Jun 19, 2024

Thank you for following up on this! If I'm understanding this correctly, the scalar seems to be coming from snnTorch. But your addition, @DylanMuir, should work with both scalars and arrays alike, correct? In that case, I think it's an improvement compared to the previous version. Would you agree @stevenabreu7?

Regarding the snnTorch codebase, I opened an issue here: jeshraghian/snntorch#334
I'm not so familiar with the codebase though, so I'm not sure I'm a good candidate to dive in for a fix.

@stevenabreu7
Copy link
Collaborator

Sorry about the delay.. As it is now, it breaks the types of 2D neuron populations. I pushed a simple change here to ensure that if v_threshold is scalar, it is cast into a numpy array, then we can also keep the v_treshold.shape at the end. Does this work for you @DylanMuir?

@stevenabreu7
Copy link
Collaborator

PR for snnTorch is also up here: jeshraghian/snntorch#335

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants