PyNN icon indicating copy to clipboard operation
PyNN copied to clipboard

Discussion: Rename `ID` to something more meaningful or remove it entirely?

Open apdavison opened this issue 4 years ago • 7 comments

The ID class provides a convenient API for modifying individual neurons in a Population.

Although current implementations store arrays of ID objects internally, this is wasteful of memory, so ideally ID objects should be created on the fly when needed.

All of the functionality of ID is also available via PopulationView, I believe (need to check), so there is an argument for removing the ID class entirely to simplify the API.

If we decide to keep it, we could also consider renaming it to something more meaningful, e.g. Cell or Neuron.

apdavison avatar Mar 12 '21 07:03 apdavison

My own opinion would be to remove ID as a thing, and just leave PopulationView. A PopulationView of 1 item is like an array of 1 item, but it simplifies things quite a bit, as it means that indexing a Population always results in a PopulationView. I believe PopulationView also acts very similarly to a Population, so it makes things more consistent there too. Finally, though probably least critically, it may be quite difficult to create Projections between single neurons without this change (though I am not sure if there are other ways to get a PopulationView of a single neuron).

rowleya avatar Mar 12 '21 08:03 rowleya

thanks for the feedback @rowleya. To get a PopulationView using a single neuron you need to use slice notation, e.g. population[n:n + 1], and this is what should be used to create Projections between single neurons. The effect of removing ID is that population[n] would also give a PopulationView of size one.

apdavison avatar Mar 12 '21 09:03 apdavison

Hmm OK, I guess this means that it works like a normal Python list in that case, which would be more of a reason to keep the ID object in some form or other. I agree that the name ID is not great; another option would be PopulationElement since this then fits the existing naming scheme.

rowleya avatar Mar 12 '21 09:03 rowleya

I too would in a favour of population[n] giving a PopulationView of size one.

Christian-B avatar Mar 12 '21 09:03 Christian-B

If others prefer to keep the concept that pop[1] returns an Element than having an Class PopulationElement works for me too.

But if we decide to do that then it must be allowed to use PopulationElement to create Projections too. All or nothing!

Christian-B avatar Mar 12 '21 09:03 Christian-B

I would just like to confirm that we support the removal of the ID class. Currently, I do not see any extra utility that the ID class provides, that is not already provided by the PopulationView. But please let me know if I am missing something. It seems to me that having all types of indexings of the Population returning PopulationView would simplify things, and prevent bugs.

On Fri, Mar 12, 2021 at 10:35 AM Christian Y. Brenninkmeijer < @.***> wrote:

If others prefer to keep the concept that pop[1] returns an Element than having an Class PopulationElement works for me too

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/NeuralEnsemble/PyNN/issues/715#issuecomment-797365277, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH3M3ALEDMXH3RQLJIWYVTTDHN5XANCNFSM4ZBZCQGA .

antolikjan avatar Mar 12 '21 10:03 antolikjan

For pynn.brainscales2 (I believe) we actively work-around the ID class and explicitly cast to int to increase speed at some places :). I'll check.

⇒ removal would be okay for us.

muffgaga avatar Mar 12 '21 17:03 muffgaga