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

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

Open
apdavison opened this issue Mar 12, 2021 · 7 comments
Open
Milestone

Comments

@apdavison
Copy link
Member

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.

@rowleya
Copy link

rowleya commented Mar 12, 2021

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).

@apdavison
Copy link
Member Author

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.

@rowleya
Copy link

rowleya commented Mar 12, 2021

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.

@Christian-B
Copy link

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

@Christian-B
Copy link

Christian-B commented Mar 12, 2021

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!

@antolikjan
Copy link
Contributor

antolikjan commented Mar 12, 2021 via email

@muffgaga
Copy link
Contributor

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.

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

No branches or pull requests

5 participants