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

Expose synapse span type choices #74

Open
tnowotny opened this issue Sep 21, 2018 · 6 comments
Open

Expose synapse span type choices #74

tnowotny opened this issue Sep 21, 2018 · 6 comments

Comments

@tnowotny
Copy link
Contributor

I have made a simple change that allows to set the default synapse span type for all synapse groups in a model via a brian 2 preference devices.genn.synapse_span_type.
It would be better if we had a method to do it per synapse population.
The simple method is realised in the expose_blocksize_prefs branch if you want to have a look, @mstimberg

@thesamovar
Copy link
Member

It would be nice if codegen targets could create extra keyword arguments in Brian itself to cover this sort of thing. What do you think @mstimberg?

@denisalevi
Copy link
Member

+1 for extra keyword arguments per Synapse / Group objects :)

@mstimberg
Copy link
Member

Some more fine-grained control would be definitely nice, I also briefly discussed this with @moritzaugustin last week. The problem that I'm seeing is to not mix device-specific details and the model itself too much. The nice thing about our current device implementation is that most of the time you'll set a bunch of preferences and device settings in the beginning, but all the rest of the model is independent of the device and does not need any changes if you change the device. Having something like Synapses(..., span_type='pre') would go against this. While we could avoid errors when switching the device by simply passing on all unknown keyword arguments to a device, and silently ignoring unknown keyword arguments on the device, I think this would still not be great for two reasons:

  1. A typo in a keyword argument (say, onpre=... instead of on_pre=...) would be quietly ignored.
  2. Users reading a code using such a keyword argument might be confused because the Brian 2 documentation does not mention the argument.

I think we only have two options, both with their advantages and disadvantages:

  1. Use the preference, but have it accept a dictionary of names and strategies, e.g. something like
    devices.genn.synapse_span_type = {'synapses': 'pre', 'synapses_1': 'post'}
    Of course this will be more meaningful/readable when assigning names manually, or writing something like:
    devices.genn.synapse_span_type = {exc_syn.name: 'pre',
                                      inh_syn.name: 'post'}
    (or just using the object references instead of their name)
  2. Use a generic device_options argument for main classes such as NeuronGroup, Synapses and maybe SpikeMonitor and StateMonitor that takes a dictionary. Keys that a device does not know about will be ignored (maybe with a warning as we do for set_device/build):
    syn = Synapses(..., device_options={'span_type': 'pre'})

The advantage of solution 1 is that it separates the device settings from the main model code, but it needs you to make use of Python's naming (or mix it in with the model code again), which most users are not really aware of. I think I prefer option 2, even though it mixes model and device implementations a bit – given that it is named device_options, I think this should at least be clear to the user.

@tnowotny
Copy link
Contributor Author

tnowotny commented Oct 5, 2018

Yes, it is a bit of a difficult construct and I also liked the clean separation of model and device-specific information. But on the other hand I would struggle to use version 1 (get the naming sorted) and I also think it would be somewhat difficult for users to keep it in check when scripts get longer and one has to change the spantype on top when introducing or changing the synapse group somewhere else.
So my preference would be the second option.

@moritzaugustin
Copy link

I also like option 2 (now) as overall user convenience seems higher. Moreover, devices still can offer global prefs (e.g. in your case span_type) that apply to all synaptic pathways at once, which for simple structured models still allows clear abstraction of device and model specification within option 2.

@tnowotny
Copy link
Contributor Author

tnowotny commented Apr 1, 2021

This could be revisited within GSoC2021.

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