ExpLeak inheriting from LIF is inconsistent about sinabs HOT 5 CLOSED

Do you mean it is semantically inconsistent because ExpLeak is not a form of LIF?
Would it make more sense to invert this oder, i.e. LIF inheriting from ExpLeak?

from sinabs.

I think that is what @sheiksadique meant.

from sinabs.

The problem with making ExpLeak the parent class is that IAF inheriting from ExpLeak is also not consistent. We should come up with a way to favour composition rather than inheritance

from sinabs.

I think the important aspect to retain is code retention.

So if we move towards a more functional approach, we could move all of the logic to functions and leave the classes to just call the appropriate function with the necessary activation functions. This way, we won't have to do too much re-writing of classes but design the class inheritance to be as logical as necessary.

This would also make it easier to use the exodus framework and classes/functions.

from sinabs.

To be honest, I am still not sure why this issue is relevant. Could someone please explain this to me? Does this affect usability, confuse users or developers, make the code less readable, ...?

I think we shouldn't rack our brains about what would be a semantically meaningful order of inheritance. There will always be different ways to interpret things.

I agree with @sheiksadique that we should focus on clean, concise and readable code. However, with respect to the more functional, modular approach, I believe we are already there. Forward pass, spike generation, surrogate gradients are already functions that get called by the different layers. The layer classes mostly take care of handling states and parameters.

In that regard, if we really need to change things, I suggest a flat hierarchy. I imagine this is congruent with @biphasic 's suggestion about composition rather than inheritance:

Let ExpLeak, IAF, LIF,... all inherit from StatefulLayer and configure their spike functions, time constants, etc. such that they yield the correct behavior. Also, the does_spike property could simply check whether the spike function of that object is 'None', so we don't have to re-define it for every class.

from sinabs.