genn-team / ml_genn

A library for deep learning with Spiking Neural Networks (SNN).
https://ml-genn.readthedocs.io
GNU Lesser General Public License v2.1
24 stars 7 forks source link

shape parameter convention in layers vs preprocessing #87

Closed FabianSchubert closed 1 week ago

FabianSchubert commented 9 months ago

The connect method of Conv2D assumes that the shape parameter of the input layer represents (height, width, channels): https://github.com/genn-team/ml_genn/blob/8822f90b46e514380b850b5ae1a343cb2f905bde/ml_genn/ml_genn/connectivity/conv_2d.py#L34

However, the preprocess_tonic_spikes function seems to follow a convention where shape[0] is the width of the input and shape[1] is the height, given that 2d event coordinates are provided: https://github.com/genn-team/ml_genn/blob/8822f90b46e514380b850b5ae1a343cb2f905bde/ml_genn/ml_genn/utils/data.py#L99-L100

This is not a bug per se, since the latter is just a preprocessing utility, but I think adhering to a consistent convention for the order of width and height would help avoiding errors down the line when building a preprocessing/training pipeline.

Edit: Overall I would prefer the convention of representing it as (height, width), as this would follow the usual matrix representation for image data.

neworderofjamie commented 1 week ago

The ordering in preprocess_tonic_spikes just reflects the order that shapes are specified in by tonic datasets e.g.

 sensor_size = (1280, 720, 2)

(from https://github.com/neuromorphs/tonic/blob/develop/tonic/datasets/tum_vie.py#L89)

I am fairly sure the indexing of spike_event_ids matches with conv_ih, conv_iw, conv_ic in the convolution layers which is what matters