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

Connection PyNEST class #2

Open
stinebuu opened this issue Nov 29, 2017 · 6 comments
Open

Connection PyNEST class #2

stinebuu opened this issue Nov 29, 2017 · 6 comments

Comments

@stinebuu
Copy link

stinebuu commented Nov 29, 2017

Now that we are going to deprecate GetStatus and SetStatus, and instead use get and set, we need to make a Connections PyNEST class, which you get when calling nest.GetConnections(). Then, if we implement get and set in the class, we can still get and set info on connections, just as we now do with GetStatus and SetStatus.

The class shouldn't be to difficult to implement, when we call GetConnections() we receive a ConnectionDatum, and then we can use this just as we use it in the GIDCollection, Mask and Parameter classes.

The class should at least have:

  • len(..)
  • ability to iterate
  • indexing
  • print(..)
  • set()
  • get()

I don't use GetConnections that often, so please add functionalities if something is missing, or remove if something is not needed.

Right now, we return a tuple with arrays on the Python level when calling GetConnections(), where each array is of the form:

  • [source-gid, target-gid, target-thread, synapse-model-id, port]

Is this how it should be when we for instance print the Connections as well? It is compact and nice, but you have to go and look at the documentation to know what the different elements represents.

How GetConnections works now:

>> n.Create('iaf_psc_alpha', 2)
>> nest.Connect(n, n) 
>> get_conn = nest.GetConnections()

>> print(get_conn)
(array('l', [1, 1, 0, 0, 0]), array('l', [1, 2, 0, 0, 1]), array('l', [2, 1, 0, 0, 0]), array('l', [2, 2, 0, 0, 1]))

>> nest.GetStatus(get_conn)[0]
{'delay': 1.0,
 'receptor': 0,
 'sizeof': 32,
 'source': 1,
 'synapse_model': <SLILiteral: static_synapse>,
 'target': 1,
 'weight': 1.0}

>> nest.SetStatus(get_conn, 'delay', 2.0)

GetConnection can take in sources, targets, synapse_model, synapse_label.

Suggestion to how Connections will look:

We can work with the Connections class almost exactly as the return value of GetConnections is now, that is, a tuple of arrays. Then we will have something like

>> n.Create('iaf_psc_alpha', 2)
>> nest.Connect(n, n) 
>> get_conn = nest.GetConnections()

>> print(get_conn)
(array('l', [1, 1, 0, 0, 0]), array('l', [1, 2, 0, 0, 1]), array('l', [2, 1, 0, 0, 0]), array('l', [2, 2, 0, 0, 1]))

We could probably also make it so that we get a list of dictionaries:

>>  print(get_conn)
({'source_gid': 1, 'target_gid': 1, 'target-thread': 0, 'synapse-model-id': 0, 'port': 0},
 {'source_gid': 1, 'target_gid': 2, 'target-thread': 0, 'synapse-model-id': 0, 'port': 1},
 {'source_gid': 2, 'target_gid': 1, 'target-thread': 0, 'synapse-model-id': 0, 'port': 0},
 {'source_gid': 2, 'target_gid': 2, 'target-thread': 0, 'synapse-model-id': 0, 'port': 1})

>> get_conn[2]
 {'source_gid': 2, 'target_gid': 1, 'target-thread': 0, 'synapse-model-id': 0, 'port': 0},

The first is simpler, but the second is maybe more informative? Though, it would make it more complicated to extract information to use in your code. There is also a lot of overhead, and if we have many connections, which is often the case, this might be troublesome. We could make a dictionary with lists?

get and set
I guess set() should work just as SetStatus(). The question is what get should return. It could return the data the way GetStatus() now returns data:

>> get_conn.get()
({'delay': 1.0,
  'receptor': 0,
  'sizeof': 32,
  'source': 1,
  'synapse_model': <SLILiteral: static_synapse>,
  'target': 1,
  'weight': 1.0},
  ...,
 {'delay': 1.0,
  'receptor': 0,
  'sizeof': 32,
  'source': 2,
  'synapse_model': <SLILiteral: static_synapse>,
  'target': 2,
  'weight': 1.0})
>> get_conn.get('delay')
(1.0, 1.0, 1.0, 1.0)
>> get_conn.set('delay', 2.0) # To see difference between delay and weight
>> nest.GetStatus(get_conn, ['delay', 'weight'])
((2.0, 1.0), (2.0, 1.0), (2.0, 1.0), (2.0, 1.0))

But it is maybe more intuitive to follow the output from GIDCollection's get(), and get a dictionary with lists. The question then is whether we should have something similar to the GID list you get with GIDCollection's get(). We could return source_gid and target_GID.

Following the discussion in #1, we could also return lists or nested lists/tuples. So, something like

get_conn.get('delay')
[2.0, 2.0, 2.0, 2.0]
>> nest.GetStatus(get_conn, ['delay', 'weight'])
[[2.0, 2.0, 2.0, 2.0], [1.0, 1.0, 1.0, 1.0]]

I don't really know what an empty get() call should return. I personally find it really useful to call GetStatus without any parameters to know what I can set, what I can know, default values of the model etc., and think we should be able to do the same with get().

Questions:

  • What should the name of the class be?
  • What should we display when iterating and printing?
  • Thoughts on what get() and get(param) should return?
@heplesser
Copy link
Member

@stinebuu Thanks for the detailed description!

Given that we typically have a large number of connections, we need to keep compactness of representation in mind. The current representation is quite wasteful (one array per connection), and a representation with one dict per connection would be worse, even though it is more informative. I think that the most efficient solution would be a NumPy array with named columns and one row per connection.

For connections, we do not have composite properties, i.e., properties that are containers. Therefore, for the sake of compactness of representation (and efficiency), I would suggest that get() on a Connections object returns a NumPy array with named columns.

@jougs
Copy link

jougs commented Nov 30, 2017

The numbers in the (array of) arrays returned by GetConnections are only an implementation detail that the user should not actually see. I chose this representation a hundred years ago due to the lack of a better idea and no time to implement a proper ConnectionDatum in PyNEST. We should really just hand a soulless Connection object to the user, on which get(), set() and all the other things @stinebuu mentioned are defined. The Connection object should just wrap a pointer to a ConnectionDatum, so the implementation of that could change in whatever ways it wants without the need to change anything on the SLI or Python level.

We could even make the return type of GetConnections a ConnCollection which could behave similar to a GIDCollection.

@stinebuu
Copy link
Author

stinebuu commented Dec 4, 2017

At the VC on 1st of December, it was decided that

  • The user is given a soulless Connectome class that wraps around the ConnectionDatums that are returned when GetConnections is called.
    • This means that iterating is not possible.
  • Slicing should be done with the GetConnections call, so if you want a subset of connections, you send in sliced GIDCollections.
  • To get any info on the connections, you have to use get().
    • This means that if you want to know for instance source or target, you call connections.get('source')/connections.get('target'), indexing is no longer allowed.
    • You can no longer print the Connectome.

The Connectome class should at first have the following methods:

  • __len__()
  • __eq__()
  • get()
  • set()

@stinebuu
Copy link
Author

stinebuu commented Dec 4, 2017

I have made a separate branch where the Connectome work is being done, https://github.com/compneuronmbu/nest-simulator/tree/Connectome . It is based off the SubnetFreeTopology branch. It currently have 26 failing tests, but I am working on it.

stinebuu pushed a commit that referenced this issue May 8, 2018
Remove old versions of scripts and small cleanup
stinebuu pushed a commit that referenced this issue Aug 21, 2018
cleanup of compute_compressed_secondary_recv_buffer_positions
stinebuu pushed a commit that referenced this issue Aug 21, 2018
@stinebuu
Copy link
Author

I am closing this issue as the Connectome branch was merged into the nest-3 branch in commit 773192e.

@heplesser
Copy link
Member

I am re-opening this, since I would like to discuss internal representation again, see also #9 and #10.

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

No branches or pull requests

3 participants